From 17e5a28380af416ae0d4ef2c9124ff72570b4a1a Mon Sep 17 00:00:00 2001 From: c-brenn Date: Mon, 11 Mar 2024 12:53:06 +0000 Subject: [PATCH 1/2] Stop `Bypass.Instance` correctly in `dispatch_awaiting_callers/1` At the moment `dispatch_awaiting_callers/1` it appears that `dispatch_awaiting_callers/1` attempts to shut down the current instance using `GenServer.stop(:normal)`. This causes a crash as the first argument to `GenServer.stop/3` is meant to be a PID or name, rather than a reason. For example: ``` ** (stop) exited in: GenServer.stop(:normal, :normal, :infinity) ** (EXIT) no process: the process is not alive or there's no process currently associated with the given name, possibly because its application isn't started (elixir 1.16.0) lib/gen_server.ex:1061: GenServer.stop/3 (bypass 2.1.0) lib/bypass/instance.ex:385: Bypass.Instance.dispatch_awaiting_callers/1 (bypass 2.1.0) lib/bypass/instance.ex:62: Bypass.Instance.handle_info/2 (stdlib 5.2) gen_server.erl:1095: :gen_server.try_handle_info/3 (stdlib 5.2) gen_server.erl:1183: :gen_server.handle_msg/6 (stdlib 5.2) proc_lib.erl:241: :proc_lib.init_p_do_apply/3 Last message: {:DOWN, #Reference<0.1124410943.1226571777.107399>, :process, #PID<0.6661.0>, :shutdown} ``` This appears to happen when the ExUnit test that spawned the bypass instance has exited, but the bypass instance is still awaiting a request. For example if the test spawned an async process that will make a request to a bypass stub. This commit refactors `dispatch_awaiting_callers/1` so that it now returns either `{:noreply, state}` or `{:stop, ...}` so that it can handle any awaiting callers and then shut down the GenServer which seems to be the intended behaviour of the current code. --- lib/bypass/instance.ex | 53 ++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/lib/bypass/instance.ex b/lib/bypass/instance.ex index 9426c00..986c6ec 100644 --- a/lib/bypass/instance.ex +++ b/lib/bypass/instance.ex @@ -59,12 +59,17 @@ defmodule Bypass.Instance do {route, state} -> result = {:exit, {:exit, reason, []}} - {:noreply, route |> put_result(ref, result, state) |> dispatch_awaiting_callers()} + + route + |> put_result(ref, result, state) + |> dispatch_awaiting_callers() end end def handle_cast({:put_expect_result, route, ref, result}, state) do - {:noreply, route |> put_result(ref, result, state) |> dispatch_awaiting_callers()} + route + |> put_result(ref, result, state) + |> dispatch_awaiting_callers() end def handle_call(request, from, state) do @@ -363,34 +368,32 @@ defmodule Bypass.Instance do {ref, put_in(state.monitors[ref], route)} end - defp dispatch_awaiting_callers( - %{ - callers_awaiting_down: down_callers, - callers_awaiting_exit: exit_callers, - socket: socket, - ref: ref - } = state - ) do + defp dispatch_awaiting_callers(state) do if retained_plugs_count(state) == 0 do - down_reset = - if length(down_callers) > 0 do - do_down(ref, socket) - Enum.each(down_callers, &GenServer.reply(&1, :ok)) - %{state | socket: nil, callers_awaiting_down: []} - end - - if length(exit_callers) > 0 do - {result, _updated_state} = do_exit(state) - Enum.each(exit_callers, &GenServer.reply(&1, result)) - GenServer.stop(:normal) - end - - down_reset || state - else state + |> handle_callers_awaiting_down() + |> handle_callers_awaiting_exit() + else + {:noreply, state} end end + defp handle_callers_awaiting_down(%{callers_awaiting_down: []} = state), do: state + + defp handle_callers_awaiting_down(%{callers_awaiting_down: down_callers} = state) do + do_down(state.ref, state.socket) + Enum.each(down_callers, &GenServer.reply(&1, :ok)) + %{state | socket: nil, callers_awaiting_down: []} + end + + defp handle_callers_awaiting_exit(%{callers_awaiting_exit: []} = state), do: {:noreply, state} + + defp handle_callers_awaiting_exit(%{callers_awaiting_exit: exit_callers} = state) do + {result, updated_state} = do_exit(state) + Enum.each(exit_callers, &GenServer.reply(&1, result)) + {:stop, :normal, result, updated_state} + end + defp retained_plugs_count(state) do state.expectations |> Map.values() From 8db6a5b147efde9eacb7924bec2a087454ecf728 Mon Sep 17 00:00:00 2001 From: c-brenn Date: Fri, 26 Apr 2024 15:24:17 +0100 Subject: [PATCH 2/2] fixup! Stop `Bypass.Instance` correctly in `dispatch_awaiting_callers/1` --- lib/bypass/instance.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bypass/instance.ex b/lib/bypass/instance.ex index 986c6ec..6d32a0f 100644 --- a/lib/bypass/instance.ex +++ b/lib/bypass/instance.ex @@ -391,7 +391,7 @@ defmodule Bypass.Instance do defp handle_callers_awaiting_exit(%{callers_awaiting_exit: exit_callers} = state) do {result, updated_state} = do_exit(state) Enum.each(exit_callers, &GenServer.reply(&1, result)) - {:stop, :normal, result, updated_state} + {:stop, :normal, updated_state} end defp retained_plugs_count(state) do