Do not deadlock Logger if handler crashes on sync mode, closes #10420

Author: josevalim

lib/logger/lib/logger/handler.ex

@@ -147,8 +147,17 @@ defmodule Logger.Handler do
     end
   end
 
-  defp notify(:sync, msg), do: :gen_event.sync_notify(Logger, msg)
-  defp notify(:async, msg), do: :gen_event.notify(Logger, msg)
+  defp notify(:sync, msg) do
+    pid = Process.whereis(Logger)
+
+    # If we are within the logger process itself,
+    #  we cannot use sync notify as that will deadlock.
+    if pid == self(), do: :gen_event.notify(pid, msg), else: :gen_event.sync_notify(pid, msg)
+  end
+
+  defp notify(:async, msg) do
+    :gen_event.notify(Logger, msg)
+  end
 
   defp truncate(data, n) when is_list(data) do
     Logger.Utils.truncate(data, n)

lib/logger/test/logger_test.exs

@@ -62,7 +62,7 @@ defmodule LoggerTest do
              Logger.add_backend({UnknownBackend, :hello})
   end
 
-  test "logs or writes to stderr on failed backends" do
+  test "logs or writes to stderr on failed call on async mode" do
     assert {:ok, _} = Logger.add_backend({MyBackend, :hello})
 
     assert capture_log(fn ->
@@ -86,6 +86,32 @@ defmodule LoggerTest do
     Logger.add_backend(:console)
   end
 
+  test "logs or writes to stderr on failed call on sync mode" do
+    Logger.configure(sync_threshold: 0)
+    assert {:ok, _} = Logger.add_backend({MyBackend, :hello})
+
+    assert capture_log(fn ->
+             :gen_event.call(Logger, {MyBackend, :hello}, :error)
+             wait_for_handler(Logger, {MyBackend, :hello})
+           end) =~
+             ":gen_event handler {LoggerTest.MyBackend, :hello} installed in Logger terminating"
+
+    assert :ok = Logger.remove_backend(:console)
+
+    assert ExUnit.CaptureIO.capture_io(:stderr, fn ->
+             :gen_event.call(Logger, {MyBackend, :hello}, :error)
+             wait_for_handler(Logger, {MyBackend, :hello})
+           end) =~
+             ":gen_event handler {LoggerTest.MyBackend, :hello} installed in Logger terminating"
+
+    # Flush logs before reattaching to avoid OTP reports
+    Logger.flush()
+  after
+    Logger.configure(sync_threshold: 20)
+    Logger.remove_backend({MyBackend, :hello})
+    Logger.add_backend(:console)
+  end
+
   test "level/0" do
     assert Logger.level() == :debug