ssh: Keep daemon alive while replacing options

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},

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.
-