diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl index 0a64807fe78c..0426577dc4f2 100644 --- a/lib/ssh/src/ssh_system_sup.erl +++ b/lib/ssh/src/ssh_system_sup.erl @@ -41,7 +41,8 @@ addresses/1, get_options/2, get_acceptor_options/1, - replace_acceptor_options/2 + replace_acceptor_options/2, + start_restart_guard/0 ]). %% Supervisor callback @@ -167,18 +168,29 @@ get_acceptor_options(SysPid) -> replace_acceptor_options(SysPid, NewOpts) -> case get_daemon_listen_address(SysPid) of {ok,Address} -> - try stop_listener(SysPid) - of - ok -> - restart_acceptor(SysPid, Address, NewOpts) - catch - error:_ -> - restart_acceptor(SysPid, Address, NewOpts) + GuardId = {?MODULE, replace_acceptor_options, make_ref()}, + GuardSpec = + #{id => GuardId, + start => {?MODULE, start_restart_guard, []}, + restart => temporary, + significant => true, + type => worker}, + case supervisor:start_child(SysPid, GuardSpec) of + {ok,_GuardPid} -> + try replace_acceptor_options(SysPid, Address, NewOpts) + after + remove_restart_guard(SysPid, GuardId) + end; + {error,Error} -> + {error,Error} end; {error,Error} -> {error,Error} end. +start_restart_guard() -> + {ok,spawn_link(fun restart_guard/0)}. + %%%========================================================================= %%% Supervisor callback %%%========================================================================= @@ -221,6 +233,30 @@ get_options(Sup, Address = #address{}) -> %%% Internal functions %%%========================================================================= +replace_acceptor_options(SysPid, Address, NewOpts) -> + try stop_listener(SysPid) + of + ok -> + restart_acceptor(SysPid, Address, NewOpts) + catch + error:_ -> + restart_acceptor(SysPid, Address, NewOpts) + end. + +remove_restart_guard(SysPid, GuardId) -> + try + _ = supervisor:terminate_child(SysPid, GuardId), + _ = supervisor:delete_child(SysPid, GuardId), + ok + catch + exit:{noproc,_} -> ok + end. + +restart_guard() -> + receive + _ -> restart_guard() + end. + %% A separate function because this spec is need in >1 places acceptor_sup_child_spec(SysSup, Address, Options) -> #{id => {ssh_acceptor_sup,Address}, diff --git a/lib/ssh/test/ssh_options_SUITE.erl b/lib/ssh/test/ssh_options_SUITE.erl index 1d679c9d313e..5692b8f16602 100644 --- a/lib/ssh/test/ssh_options_SUITE.erl +++ b/lib/ssh/test/ssh_options_SUITE.erl @@ -85,6 +85,7 @@ raw_option/1, config_file/1, config_file_modify_algorithms_order/1, + daemon_replace_options_last_connection_race/1, daemon_replace_options_simple/1, daemon_replace_options_algs/1, daemon_replace_options_algs_connect/1, @@ -156,6 +157,7 @@ all() -> raw_option, config_file, config_file_modify_algorithms_order, + daemon_replace_options_last_connection_race, daemon_replace_options_simple, daemon_replace_options_algs, daemon_replace_options_algs_connect, @@ -1867,6 +1869,66 @@ config_file_modify_algorithms_order(Config) -> end. +%%-------------------------------------------------------------------- +daemon_replace_options_last_connection_race(Config) -> + {DaemonRef, Host, Port} = ssh_test_lib:std_daemon(Config, []), + ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, []), + ConnectionSup = connection_sup(DaemonRef), + TestPid = self(), + PauseRef = make_ref(), + DebugFun = + fun(armed, + {in, {'$gen_call', _, + {delete_child, {ssh_acceptor_sup, _}}}}, _) -> + TestPid ! {listener_stopped, PauseRef}, + receive + {continue_restart, PauseRef} -> triggered + end; + (State, _, _) -> + State + end, + ok = sys:install(DaemonRef, {DebugFun, armed}), + spawn_link( + fun() -> + Result = + try ssh:daemon_replace_options(DaemonRef, []) of + Reply -> Reply + catch + exit:Reason -> {exit, Reason} + end, + TestPid ! {replace_result, PauseRef, Result} + end), + receive + {listener_stopped, PauseRef} -> ok + end, + ok = ssh:close(ConnectionRef), + ok = wait_for_exit_message(DaemonRef, ConnectionSup, 100), + DaemonRef ! {continue_restart, PauseRef}, + receive + {replace_result, PauseRef, Result} -> + {ok, DaemonRef} = Result + end, + true = is_process_alive(DaemonRef), + {ok, _} = ssh:daemon_info(DaemonRef). + +connection_sup(DaemonRef) -> + [{_, ConnectionSup, supervisor, [ssh_connection_sup]}] = + [Child || Child = {_, _, supervisor, [ssh_connection_sup]} <- + supervisor:which_children(DaemonRef)], + ConnectionSup. + +wait_for_exit_message(_, _, 0) -> + {error, timeout}; +wait_for_exit_message(DaemonRef, ConnectionSup, Attempts) -> + {messages, Messages} = process_info(DaemonRef, messages), + case lists:keyfind(ConnectionSup, 2, Messages) of + {'EXIT', ConnectionSup, _} -> + ok; + false -> + timer:sleep(10), + wait_for_exit_message(DaemonRef, ConnectionSup, Attempts - 1) + end. + %%-------------------------------------------------------------------- daemon_replace_options_simple(Config) -> SysDir = proplists:get_value(data_dir, Config), @@ -2120,4 +2182,3 @@ test_not_connect(Config, Host, Port, Opts) -> catch error:{badmatch, {error,_}} -> ok end. -