From 7a3aa3b225845b007703621c408f02adb3d2699c Mon Sep 17 00:00:00 2001 From: Jan Uhlig Date: Tue, 25 Jun 2024 10:30:28 +0200 Subject: [PATCH 1/4] Fix halfway-stopping of listeners * if the process calling ranch:stop_listener crashes before finishing, the stopping is still exetuted completely * if a listener is terminated but not deleted, calling ranch:stop_listener removes the remnant --- src/ranch.erl | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/src/ranch.erl b/src/ranch.erl index d48c18a6fc..d909afd735 100644 --- a/src/ranch.erl +++ b/src/ranch.erl @@ -193,24 +193,42 @@ start_error(_, Error) -> Error. -spec stop_listener(ref()) -> ok | {error, not_found}. stop_listener(Ref) -> + Parent = self(), + Tag = make_ref(), + {StopperPid, StopperMon} = spawn_monitor(fun() -> Parent ! {Tag, stop_listener1(Ref)} end), + receive + {Tag, Result} -> + demonitor(StopperMon, [flush]), + Result; + {'DOWN', StopperMon, process, StopperPid, Error} -> + {error, Error} + end. + +stop_listener1(Ref) -> + TransportAndOpts = maybe_get_transport_and_opts(Ref), + case supervisor:terminate_child(ranch_sup, {ranch_listener_sup, Ref}) of + ok -> + _ = supervisor:delete_child(ranch_sup, {ranch_listener_sup, Ref}), + ranch_server:cleanup_listener_opts(Ref), + stop_listener2(TransportAndOpts); + {error, _} = Error -> + Error + end. + +stop_listener2({Transport, TransOpts}) -> + Transport:cleanup(TransOpts), + ok; +stop_listener2(undefined) -> + ok. + +maybe_get_transport_and_opts(Ref) -> try - [_, Transport0, _, _, _] = ranch_server:get_listener_start_args(Ref), - TransOpts0 = get_transport_options(Ref), - {Transport0, TransOpts0} - of - {Transport, TransOpts} -> - case supervisor:terminate_child(ranch_sup, {ranch_listener_sup, Ref}) of - ok -> - _ = supervisor:delete_child(ranch_sup, {ranch_listener_sup, Ref}), - ranch_server:cleanup_listener_opts(Ref), - Transport:cleanup(TransOpts), - ok; - {error, Reason} -> - {error, Reason} - end + [_, Transport, _, _, _] = ranch_server:get_listener_start_args(Ref), + TransOpts = get_transport_options(Ref), + {Transport, TransOpts} catch error:badarg -> - {error, not_found} + undefined end. -spec suspend_listener(ref()) -> ok | {error, any()}. From a2c959e0b45437c7ecab8a4a511ca1d9f25edf32 Mon Sep 17 00:00:00 2001 From: Jan Uhlig Date: Thu, 4 Jul 2024 10:34:42 +0200 Subject: [PATCH 2/4] Use erpc:call instead of custom call mechanism --- src/ranch.erl | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/ranch.erl b/src/ranch.erl index d909afd735..cb37690c52 100644 --- a/src/ranch.erl +++ b/src/ranch.erl @@ -193,16 +193,11 @@ start_error(_, Error) -> Error. -spec stop_listener(ref()) -> ok | {error, not_found}. stop_listener(Ref) -> - Parent = self(), - Tag = make_ref(), - {StopperPid, StopperMon} = spawn_monitor(fun() -> Parent ! {Tag, stop_listener1(Ref)} end), - receive - {Tag, Result} -> - demonitor(StopperMon, [flush]), - Result; - {'DOWN', StopperMon, process, StopperPid, Error} -> - {error, Error} - end. + %% We need to provide an integer timeout to erpc:call, + %% otherwise the function will be executed in the calling + %% process. 16#ffffffff is as close to 'infinity' as we + %% can get. + erpc:call(node(), fun() -> stop_listener1(Ref) end, 16#ffffffff). stop_listener1(Ref) -> TransportAndOpts = maybe_get_transport_and_opts(Ref), From d9a9521403be61515e786791e5b2347ac22b9370 Mon Sep 17 00:00:00 2001 From: Jan Uhlig Date: Thu, 4 Jul 2024 13:50:48 +0200 Subject: [PATCH 3/4] Remove explicit cleanup from stopping listeners --- src/ranch.erl | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/ranch.erl b/src/ranch.erl index cb37690c52..4e0f09f624 100644 --- a/src/ranch.erl +++ b/src/ranch.erl @@ -195,20 +195,15 @@ start_error(_, Error) -> Error. stop_listener(Ref) -> %% We need to provide an integer timeout to erpc:call, %% otherwise the function will be executed in the calling - %% process. 16#ffffffff is as close to 'infinity' as we - %% can get. - erpc:call(node(), fun() -> stop_listener1(Ref) end, 16#ffffffff). + %% process. 5 minutes should be enough. + erpc:call(node(), fun() -> stop_listener1(Ref) end, 300000). stop_listener1(Ref) -> TransportAndOpts = maybe_get_transport_and_opts(Ref), - case supervisor:terminate_child(ranch_sup, {ranch_listener_sup, Ref}) of - ok -> - _ = supervisor:delete_child(ranch_sup, {ranch_listener_sup, Ref}), - ranch_server:cleanup_listener_opts(Ref), - stop_listener2(TransportAndOpts); - {error, _} = Error -> - Error - end. + _ = supervisor:terminate_child(ranch_sup, {ranch_listener_sup, Ref}), + Result = supervisor:delete_child(ranch_sup, {ranch_listener_sup, Ref}), + ok = stop_listener2(TransportAndOpts), + Result. stop_listener2({Transport, TransOpts}) -> Transport:cleanup(TransOpts), From b2bbc4908b67252aac4348acfc15ae4711bcdf01 Mon Sep 17 00:00:00 2001 From: Jan Uhlig Date: Thu, 4 Jul 2024 16:04:29 +0200 Subject: [PATCH 4/4] Augment explanation for the local use of erpc:call --- src/ranch.erl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ranch.erl b/src/ranch.erl index 4e0f09f624..81e63f6ee3 100644 --- a/src/ranch.erl +++ b/src/ranch.erl @@ -193,6 +193,11 @@ start_error(_, Error) -> Error. -spec stop_listener(ref()) -> ok | {error, not_found}. stop_listener(Ref) -> + %% The stop procedure must be executed in a separate + %% process to make sure that it won't be interrupted + %% in the middle in case the calling process crashes. + %% We use erpc:call locally so we don't have to + %% implement a custom spawn/call mechanism. %% We need to provide an integer timeout to erpc:call, %% otherwise the function will be executed in the calling %% process. 5 minutes should be enough.