From e7dfd84f00ceded6261a8592e428e34eb3575887 Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Wed, 19 Mar 2025 15:25:33 +0100 Subject: [PATCH 1/2] Prevent exits from leaking in ExUnit.CaptureServer When a process that traps exits used ExUnit's log capture functionality, it would receive stray exits from the Tasks started inside it that could lead to issues if not handled. We fix this by disabling exit trapping while awaiting the tasks in the CaptureServer's log handler. --- lib/ex_unit/lib/ex_unit/capture_server.ex | 4 ++++ lib/ex_unit/test/ex_unit/capture_log_test.exs | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/ex_unit/lib/ex_unit/capture_server.ex b/lib/ex_unit/lib/ex_unit/capture_server.ex index 2c9269b133c..a552e5359eb 100644 --- a/lib/ex_unit/lib/ex_unit/capture_server.ex +++ b/lib/ex_unit/lib/ex_unit/capture_server.ex @@ -231,6 +231,8 @@ defmodule ExUnit.CaptureServer do ## :logger handler callback. def log(event, _config) do + trap_exits = Process.flag(:trap_exit, false) + :ets.tab2list(@ets) |> Enum.filter(fn {_ref, _string_io, level, _formatter_mod, _formatter_config} -> :logger.compare_levels(event.level, level) in [:gt, :eq] @@ -255,6 +257,8 @@ defmodule ExUnit.CaptureServer do end) |> Task.await_many(:infinity) + Process.flag(:trap_exit, trap_exits) + :ok end end diff --git a/lib/ex_unit/test/ex_unit/capture_log_test.exs b/lib/ex_unit/test/ex_unit/capture_log_test.exs index 4d3dd49c206..18bdea7bc98 100644 --- a/lib/ex_unit/test/ex_unit/capture_log_test.exs +++ b/lib/ex_unit/test/ex_unit/capture_log_test.exs @@ -95,6 +95,16 @@ defmodule ExUnit.CaptureLogTest do end) end + test "exits don't leak" do + Process.flag(:trap_exit, true) + + capture_log(fn -> + Logger.error("oh no!") + end) + + refute_receive {:EXIT, _, _} + end + describe "with_log/2" do test "returns the result and the log" do {result, log} = From 25576cc6cb8e019edb7e3ee65c6cca5692b1859b Mon Sep 17 00:00:00 2001 From: Steffen Deusch Date: Wed, 19 Mar 2025 16:27:57 +0100 Subject: [PATCH 2/2] don't change trap_exit, just consume messages --- lib/ex_unit/lib/ex_unit/capture_server.ex | 56 +++++++++++++---------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/lib/ex_unit/lib/ex_unit/capture_server.ex b/lib/ex_unit/lib/ex_unit/capture_server.ex index a552e5359eb..4ce0bdb45c6 100644 --- a/lib/ex_unit/lib/ex_unit/capture_server.ex +++ b/lib/ex_unit/lib/ex_unit/capture_server.ex @@ -231,33 +231,41 @@ defmodule ExUnit.CaptureServer do ## :logger handler callback. def log(event, _config) do - trap_exits = Process.flag(:trap_exit, false) - - :ets.tab2list(@ets) - |> Enum.filter(fn {_ref, _string_io, level, _formatter_mod, _formatter_config} -> - :logger.compare_levels(event.level, level) in [:gt, :eq] - end) - |> Enum.group_by( - fn {_ref, _string_io, _level, formatter_mod, formatter_config} -> - {formatter_mod, formatter_config} - end, - fn {_ref, string_io, _level, _formatter_mod, _formatter_config} -> - string_io - end - ) - |> Enum.map(fn {{formatter_mod, formatter_config}, string_ios} -> - Task.async(fn -> - chardata = formatter_mod.format(event, formatter_config) - - # Simply send, do not wait for reply - for string_io <- string_ios do - send(string_io, {:io_request, self(), make_ref(), {:put_chars, :unicode, chardata}}) + {:trap_exit, trapping_exits?} = Process.info(self(), :trap_exit) + + tasks = + :ets.tab2list(@ets) + |> Enum.filter(fn {_ref, _string_io, level, _formatter_mod, _formatter_config} -> + :logger.compare_levels(event.level, level) in [:gt, :eq] + end) + |> Enum.group_by( + fn {_ref, _string_io, _level, formatter_mod, formatter_config} -> + {formatter_mod, formatter_config} + end, + fn {_ref, string_io, _level, _formatter_mod, _formatter_config} -> + string_io end + ) + |> Enum.map(fn {{formatter_mod, formatter_config}, string_ios} -> + Task.async(fn -> + chardata = formatter_mod.format(event, formatter_config) + + # Simply send, do not wait for reply + for string_io <- string_ios do + send(string_io, {:io_request, self(), make_ref(), {:put_chars, :unicode, chardata}}) + end + end) end) - end) - |> Task.await_many(:infinity) - Process.flag(:trap_exit, trap_exits) + Task.await_many(tasks) + + if trapping_exits? do + for %{pid: pid} <- tasks do + receive do + {:EXIT, ^pid, _} -> :ok + end + end + end :ok end