Add TmpDir.await_cleanup/1 for reliable test synchronization (#86)

Add a GenServer call that waits for cleanup to complete after a
monitored process exits. If the process is still monitored, the
reply is deferred until the :DOWN message is processed. This
eliminates the race condition where :DOWN delivery to the GenServer
could be delayed.

Author: ericmj

lib/hexdocs/tmp_dir.ex

@@ -32,6 +32,10 @@ defmodule Hexdocs.TmpDir do
     dir
   end
 
+  def await_cleanup(pid) do
+    GenServer.call(__MODULE__, {:await_cleanup, pid}, 5000)
+  end
+
   defp track(path) do
     pid = self()
     :ets.insert(@table, {pid, path})
@@ -42,7 +46,7 @@ defmodule Hexdocs.TmpDir do
   def init(_opts) do
     Process.flag(:trap_exit, true)
     :ets.new(@table, [:named_table, :duplicate_bag, :public])
-    {:ok, %{monitors: MapSet.new()}}
+    {:ok, %{monitors: MapSet.new(), waiters: %{}}}
   end
 
   @impl true
@@ -55,10 +59,29 @@ defmodule Hexdocs.TmpDir do
     end
   end
 
+  def handle_call({:await_cleanup, pid}, from, state) do
+    if pid in state.monitors do
+      waiters = Map.update(state.waiters, pid, [from], &[from | &1])
+      {:noreply, %{state | waiters: waiters}}
+    else
+      {:reply, :ok, state}
+    end
+  end
+
   @impl true
   def handle_info({:DOWN, _ref, :process, pid, _reason}, state) do
     cleanup_pid(pid)
-    {:noreply, %{state | monitors: MapSet.delete(state.monitors, pid)}}
+
+    for from <- Map.get(state.waiters, pid, []) do
+      GenServer.reply(from, :ok)
+    end
+
+    {:noreply,
+     %{
+       state
+       | monitors: MapSet.delete(state.monitors, pid),
+         waiters: Map.delete(state.waiters, pid)
+     }}
   end
 
   @impl true

test/hexdocs/tmp_dir_test.exs

@@ -22,9 +22,8 @@ defmodule Hexdocs.TmpDirTest do
         send(test_pid, {:paths, file, dir})
       end)
 
-    ref = Process.monitor(task_pid)
     assert_receive {:paths, file, dir}
-    wait_for_cleanup(task_pid, ref)
+    Hexdocs.TmpDir.await_cleanup(task_pid)
 
     refute File.exists?(file)
     refute File.exists?(dir)
@@ -42,9 +41,8 @@ defmodule Hexdocs.TmpDirTest do
         raise "crash"
       end)
 
-    ref = Process.monitor(task_pid)
     assert_receive {:paths, file, dir}
-    wait_for_cleanup(task_pid, ref)
+    Hexdocs.TmpDir.await_cleanup(task_pid)
 
     refute File.exists?(file)
     refute File.exists?(dir)
@@ -65,9 +63,8 @@ defmodule Hexdocs.TmpDirTest do
         send(test_pid, {:paths, paths})
       end)
 
-    ref = Process.monitor(task_pid)
     assert_receive {:paths, paths}
-    wait_for_cleanup(task_pid, ref)
+    Hexdocs.TmpDir.await_cleanup(task_pid)
 
     for {file, dir} <- paths do
       refute File.exists?(file)
@@ -82,10 +79,4 @@ defmodule Hexdocs.TmpDirTest do
     assert File.exists?(file)
     assert File.dir?(dir)
   end
-
-  defp wait_for_cleanup(task_pid, ref) do
-    assert_receive {:DOWN, ^ref, :process, ^task_pid, _}, 5000
-    # Sync with the GenServer to ensure the :DOWN cleanup has been processed
-    :sys.get_state(Hexdocs.TmpDir)
-  end
 end