File 3822-ssh-Refactor-for-changes-later.patch of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 3822-ssh-Refactor-for-changes-later.patch of Package erlang

From 401f20b46ffa8c6b19d3d7233d058de616e63ff1 Mon Sep 17 00:00:00 2001
From: Hans Nilsson <hans@erlang.org>
Date: Tue, 26 Jan 2021 13:58:05 +0100
Subject: [PATCH 2/2] ssh: Refactor for changes later

---
 lib/ssh/src/ssh.erl                    | 159 +++++++++++--------------
 lib/ssh/src/ssh_acceptor.erl           |  10 +-
 lib/ssh/src/ssh_connection_handler.erl |  52 ++++----
 lib/ssh/src/ssh_connection_sup.erl     |   2 +-
 lib/ssh/src/sshd_sup.erl               |   5 +-
 lib/ssh/test/ssh_sup_SUITE.erl         |   6 +-
 6 files changed, 107 insertions(+), 127 deletions(-)

diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl
index 9abb0fdebb..22fc8f40be 100644
--- a/lib/ssh/src/ssh.erl
+++ b/lib/ssh/src/ssh.erl
@@ -144,7 +144,7 @@ connect(Socket, UserOptions, NegotiationTimeout) when is_list(UserOptions) ->
 	Options = #{} ->
             case valid_socket_to_use(Socket, ?GET_OPT(transport,Options)) of
                 ok ->
-                    ssh_connection_handler:start_connection(client, Socket, Options, NegotiationTimeout);
+                    ssh_connection_handler:start_link(client, Socket, Options, NegotiationTimeout);
                 {error,SockError} ->
                     {error,SockError}
             end
@@ -165,19 +165,8 @@ connect(Host0, Port, UserOptions, NegotiationTimeout) when is_integer(Port),
             {error, Reason};
         
         Options ->
-            SockOpts = ?GET_OPT(socket_options, Options),
-            Host = mangle_connect_address(Host0, SockOpts),
-            {_, Callback, _} = ?GET_OPT(transport, Options),
-            SocketOpts = [{active,false} | SockOpts],
-            ConnectionTimeout = ?GET_OPT(connect_timeout, Options),
-            try Callback:connect(Host, Port, SocketOpts, ConnectionTimeout) of
-                {ok, Socket} ->
-                    ssh_connection_handler:start_connection(client, Socket, Options, NegotiationTimeout);
-                {error, Reason} ->
-                    {error, Reason}
-            catch
-                _:Error -> {error, Error}
-            end
+            Host = mangle_connect_address(Host0, Options),
+            ssh_connection_handler:start_link(client, Host, Port, Options, NegotiationTimeout)
     end.
 
 %%--------------------------------------------------------------------
@@ -260,42 +249,47 @@ daemon(Port, UserOptions) when 0 =< Port,Port =< 65535 ->
     daemon(any, Port, UserOptions);
 
 daemon(Socket, UserOptions) ->
-    try
-        #{} = Options = ssh_options:handle_options(server, UserOptions),
-
-        case valid_socket_to_use(Socket, ?GET_OPT(transport,Options)) of
-            ok ->
-                {ok, {IP,Port}} = inet:sockname(Socket),
-                finalize_start(IP, Port, ?GET_OPT(profile, Options),
-                               ?PUT_INTERNAL_OPT({connected_socket, Socket}, Options),
-                               fun(Opts, DefaultResult) ->
-                                       try ssh_acceptor:handle_established_connection(
-                                             IP, Port, Opts, Socket)
-                                       of
-                                           {error,Error} ->
-                                               {error,Error};
-                                           _ ->
-                                               DefaultResult
-                                       catch
-                                           C:R ->
-                                               {error,{could_not_start_connection,{C,R}}}
-                                       end
-                               end);
-            {error,SockError} ->
-                {error,SockError}
-            end
-    catch
-        throw:bad_fd ->
-            {error,bad_fd};
-        throw:bad_socket ->
-            {error,bad_socket};
-        error:{badmatch,{error,Error}} ->
-            {error,Error};
-        error:Error ->
-            {error,Error};
-        _C:_E ->
-            {error,{cannot_start_daemon,_C,_E}}
-    end.
+    case ssh_options:handle_options(server, UserOptions) of
+        #{} = Options0 ->
+            case valid_socket_to_use(Socket, ?GET_OPT(transport,Options0)) of
+                ok ->
+                    try
+                        Options = ?PUT_INTERNAL_OPT({connected_socket, Socket}, Options0),
+                        %% throws error:Error if no usable hostkey is found
+                        ssh_connection_handler:available_hkey_algorithms(server, Options),
+                        {ok, {IP,Port}} = inet:sockname(Socket),
+                        case sshd_sup:start_child(IP, Port, Options) of
+                            Result = {ok,_} ->
+                                case ssh_connection_handler:start_link(server, Socket, Options,
+                                                                       ?GET_OPT(negotiation_timeout,Options))
+                                of
+                                    {error,Error} ->
+                                        {error,Error};
+                                    _ ->
+                                        Result
+                                end;
+                            {error, {already_started, _}} ->
+                                {error, eaddrinuse};
+                            {error, Error} ->
+                                {error, Error}
+                        end
+                    catch
+                        error:{shutdown,Err} ->
+                            {error,Err};
+                        exit:{noproc, _} ->
+                            {error, ssh_not_started};
+                        C:R ->
+                            {error,{could_not_start_connection,{C,R}}}
+                    end;
+
+                {error,SockError} ->
+                    {error,SockError}
+            end;
+
+        {error,OptionError} ->
+            {error,OptionError}
+        end.
+
 
 
 -spec daemon(any | inet:ip_address(), inet:port_number(), daemon_options()) -> {ok,daemon_ref()} | {error,term()}
@@ -307,31 +301,39 @@ daemon(Host0, Port0, UserOptions0) when 0 =< Port0, Port0 =< 65535,
     try
         {Host1, UserOptions} = handle_daemon_args(Host0, UserOptions0),
         #{} = Options0 = ssh_options:handle_options(server, UserOptions),
-        {open_listen_socket(Host1, Port0, Options0), Options0}
+        open_listen_socket(Host1, Port0, Options0)
     of
-        {{{Host,Port}, ListenSocket}, Options1} ->
+        {Host, Port, ListenSocket, Options1} ->
             try
                 %% Now Host,Port is what to use for the supervisor to register its name,
                 %% and ListenSocket is for listening on connections. But it is still owned
                 %% by self()...
-                finalize_start(Host, Port, ?GET_OPT(profile, Options1),
-                               ?PUT_INTERNAL_OPT({lsocket,{ListenSocket,self()}}, Options1),
-                               fun(Opts, Result) ->
-                                       {_, Callback, _} = ?GET_OPT(transport, Opts),
-                                       receive
-                                           {request_control, ListenSocket, ReqPid} ->
-                                               ok = Callback:controlling_process(ListenSocket, ReqPid),
-                                               ReqPid ! {its_yours,ListenSocket},
-                                               Result
-                                       end
-                               end)
+
+                %% throws error:Error if no usable hostkey is found
+                ssh_connection_handler:available_hkey_algorithms(server, Options1),
+                sshd_sup:start_child(Host, Port, Options1)
             of
-                {error,Err} ->
+                Result = {ok,_} ->
+                    receive
+                        {request_control, ListenSocket, ReqPid} ->
+                            {_, Callback, _} = ?GET_OPT(transport, Options1),
+                            ok = Callback:controlling_process(ListenSocket, ReqPid),
+                            ReqPid ! {its_yours,ListenSocket}
+                    end,
+                    Result;
+                {error, {already_started, _}} ->
                     close_listen_socket(ListenSocket, Options1),
-                    {error,Err};
-                OK ->
-                    OK
+                    {error, eaddrinuse};
+                {error, Error} ->
+                    close_listen_socket(ListenSocket, Options1),
+                    {error, Error}
             catch
+                error:{shutdown,Err} ->
+                    close_listen_socket(ListenSocket, Options1),
+                    {error,Err};
+                exit:{noproc, _} ->
+                    close_listen_socket(ListenSocket, Options1),
+                    {error, ssh_not_started};
                 error:Error ->
                     close_listen_socket(ListenSocket, Options1),
                     error(Error);
@@ -760,7 +762,7 @@ open_listen_socket(_Host0, Port0, Options0) ->
                 ssh_acceptor:listen(0, Options0)
         end,
     {ok,{LHost,LPort}} = inet:sockname(LSock),
-    {{LHost,LPort}, LSock}.
+    {LHost, LPort, LSock, ?PUT_INTERNAL_OPT({lsocket,{LSock,self()}}, Options0)}.
 
 %%%----------------------------------------------------------------
 close_listen_socket(ListenSocket, Options) ->
@@ -771,27 +773,6 @@ close_listen_socket(ListenSocket, Options) ->
         _C:_E -> ok
     end.
 
-%%%----------------------------------------------------------------
-finalize_start(Host, Port, Profile, Options0, F) ->
-    try
-        %% throws error:Error if no usable hostkey is found
-        ssh_connection_handler:available_hkey_algorithms(server, Options0),
-
-        sshd_sup:start_child(Host, Port, Profile, Options0)
-    of
-        {error, {already_started, _}} ->
-            {error, eaddrinuse};
-        {error, Error} ->
-            {error, Error};
-        Result = {ok,_} ->
-            F(Options0, Result)
-    catch
-        error:{shutdown,Err} ->
-            {error,Err};
-        exit:{noproc, _} ->
-            {error, ssh_not_started}
-    end.
-
 %%%----------------------------------------------------------------
 map_ip(Fun, {address,IP}) when is_tuple(IP) ->
     Fun(IP);
diff --git a/lib/ssh/src/ssh_acceptor.erl b/lib/ssh/src/ssh_acceptor.erl
index 773d8025cb..7e591fb9d9 100644
--- a/lib/ssh/src/ssh_acceptor.erl
+++ b/lib/ssh/src/ssh_acceptor.erl
@@ -27,8 +27,7 @@
 %% Internal application API
 -export([start_link/4,
 	 number_of_connections/1,
-	 listen/2,
-	 handle_established_connection/4]).
+	 listen/2]).
 
 %% spawn export  
 -export([acceptor_init/5, acceptor_loop/6]).
@@ -112,11 +111,6 @@ listen(Port, Options) ->
 	    Other
     end.
 
-%%%----------------------------------------------------------------
-handle_established_connection(Address, Port, Options, Socket) ->
-    {_, Callback, _} = ?GET_OPT(transport, Options),
-    handle_connection(Callback, Address, Port, Options, Socket).
-
 %%--------------------------------------------------------------------
 %%% Internal functions
 %%--------------------------------------------------------------------
@@ -191,7 +185,7 @@ handle_connection(Callback, Address, Port, Options, Socket) ->
     case number_of_connections(SystemSup) < MaxSessions of
 	true ->
 	    NegTimeout = ?GET_OPT(negotiation_timeout, Options),
-            ssh_connection_handler:start_connection(server, {Address,Port}, Socket, Options, NegTimeout);
+            ssh_connection_handler:start_link(server, Address, Port, Socket, Options, NegTimeout);
 	false ->
 	    Callback:close(Socket),
 	    IPstr = if is_tuple(Address) -> inet:ntoa(Address);
diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl
index 070039b309..870ce97333 100644
--- a/lib/ssh/src/ssh_connection_handler.erl
+++ b/lib/ssh/src/ssh_connection_handler.erl
@@ -40,8 +40,7 @@
 %%====================================================================
 
 %%% Start and stop
--export([start_link/3,
-         start_connection/4, start_connection/5,
+-export([start_link/4, start_link/5, start_link/6,
          socket_control/3,
 	 stop/1
 	]).
@@ -98,12 +97,26 @@
 %% Start / stop
 %%====================================================================
 
-%%--------------------------------------------------------------------
-start_connection(Role, Socket, Options, NegotiationTimeout) ->
+start_link(client, Host, Port, Options, NegotiationTimeout) ->
+    {_, Callback, _} = ?GET_OPT(transport, Options),
+    SocketOpts = [{active,false} | ?GET_OPT(socket_options,Options)],
+    try Callback:connect(Host, Port, SocketOpts, ?GET_OPT(connect_timeout,Options)) of
+        {ok, Socket} ->
+            start_link(client, Socket, Options, NegotiationTimeout);
+        {error, Reason} ->
+            {error, Reason}
+    catch
+        _:badarg -> {error, {options,?GET_OPT(socket_options,Options)}};
+        _:{error,Reason} -> {error,Reason};
+        error:Error -> {error,Error};
+        Class:Error -> {error, {Class,Error}}
+    end.
+
+start_link(Role, Socket, Options, NegotiationTimeout) ->
     {ok, {Host,Port}} = inet:sockname(Socket),
-    start_connection(Role, {Host,Port}, Socket, Options, NegotiationTimeout).
+    start_link(Role, Host, Port, Socket, Options, NegotiationTimeout).
 
-start_connection(Role, {Host,Port}, Socket, Options0, NegotiationTimeout) ->
+start_link(Role, Host, Port, Socket, Options0, NegotiationTimeout) ->
     try
         Options1 = ?PUT_INTERNAL_OPT([{user_pid, self()}
                                      ], Options0),
@@ -118,7 +131,9 @@ start_connection(Role, {Host,Port}, Socket, Options0, NegotiationTimeout) ->
                                                     {subsystem_sup, SubSysSup},
                                                     {connection_sup, ConnectionSup}]}
                                     ], Options1),
-        case ssh_connection_sup:start_child(ConnectionSup, [Role, Socket, Options]) of
+        case ssh_connection_sup:start_child(ConnectionSup,
+                                            [?MODULE, [Role, Socket, Options], [{spawn_opt, [{message_queue_data,off_heap}]}]]
+                                           ) of
             {ok, Pid} ->
                 case socket_control(Socket, Pid, Options) of
                     ok ->
@@ -131,22 +146,11 @@ start_connection(Role, {Host,Port}, Socket, Options0, NegotiationTimeout) ->
         end
     catch
         exit:{noproc,{gen_server,call,_}} -> {error, ssh_not_started};
-        error:Error ->  {error, Error};
-        Class:Error ->  {error, {Class,Error}}
+        _:{error,Error} -> {error,Error};
+        error:Error -> {error,Error};
+        Class:Error -> {error, {Class,Error}}
     end.
 
-%%--------------------------------------------------------------------
--spec start_link(role(),
-		 gen_tcp:socket(),
-                 internal_options()
-		) -> {ok, pid()} | ignore | {error, term()} .
-
-start_link(Role, Socket, Options) when Role==client ; Role==server ->
-    gen_statem:start_link(?MODULE, [Role, Socket, Options],
-                          [{spawn_opt, [{message_queue_data,off_heap}]}
-                          ]).
-
-
 %%--------------------------------------------------------------------
 -spec stop(connection_ref()
 	  ) -> ok | {error, term()}.
@@ -462,12 +466,12 @@ init([Role, Socket, Opts]) when Role==client ; Role==server ->
                 process_flag(trap_exit, true),
                 {ok, {hello,Role}, D}
             catch
-                _:Error ->
-                    {stop, Error}
+                _:{error,Error} ->
+                    {stop, {error,Error}}
             end;
 
         {error,Error} ->
-            {stop, Error}
+            {stop, {error,Error}}
     end.
 
 %%%----------------------------------------------------------------
diff --git a/lib/ssh/src/ssh_connection_sup.erl b/lib/ssh/src/ssh_connection_sup.erl
index 79804b8630..192ce6f65d 100644
--- a/lib/ssh/src/ssh_connection_sup.erl
+++ b/lib/ssh/src/ssh_connection_sup.erl
@@ -51,7 +51,7 @@ init(_) ->
                  period    => 3600
                 },
     ChildSpecs = [#{id       => undefined, % As simple_one_for_one is used.
-                    start    => {ssh_connection_handler, start_link, []},
+                    start    => {gen_statem, start_link, []},
                     restart  => temporary % because there is no way to restart a crashed connection
                    }
                  ],
diff --git a/lib/ssh/src/sshd_sup.erl b/lib/ssh/src/sshd_sup.erl
index bda156223e..6fd36bf97a 100644
--- a/lib/ssh/src/sshd_sup.erl
+++ b/lib/ssh/src/sshd_sup.erl
@@ -30,7 +30,7 @@
 -include("ssh.hrl").
 
 -export([start_link/0,
-         start_child/4, 
+         start_child/3,
          start_system_subsystem/4,
          stop_child/1,
 	 stop_child/3
@@ -47,7 +47,8 @@
 start_link() ->
     supervisor:start_link({local,?SSHD_SUP}, ?MODULE, []).
 
-start_child(Address, Port, Profile, Options) ->
+start_child(Address, Port, Options) ->
+    Profile = ?GET_OPT(profile,Options),
     case ssh_system_sup:system_supervisor(Address, Port, Profile) of
        undefined ->
             %% Here we start listening on a new Host/Port/Profile
diff --git a/lib/ssh/test/ssh_sup_SUITE.erl b/lib/ssh/test/ssh_sup_SUITE.erl
index fa9dbe1c7d..dc98325c9f 100644
--- a/lib/ssh/test/ssh_sup_SUITE.erl
+++ b/lib/ssh/test/ssh_sup_SUITE.erl
@@ -368,7 +368,7 @@ chk_empty_con_daemon(Daemon) ->
 		[ConnectionSup,ChannelSup]),
     ?wait_match([{{ssh_acceptor_sup,_,_,_},_,worker,[ssh_acceptor]}],
 		supervisor:which_children(AccSup)),
-    ?wait_match([{_, _, worker,[ssh_connection_handler]}],
+    ?wait_match([{_, _, worker,[gen_statem]}],
 		supervisor:which_children(ConnectionSup)),
     ?wait_match([], supervisor:which_children(ChannelSup)),
     [ChannelSup, ConnectionSup, SubSysSup, AccSup].
@@ -404,7 +404,7 @@ check_sshd_system_tree(Daemon, Host, Port, Config) ->
     ?wait_match([{{ssh_acceptor_sup,_,_,_},_,worker,[ssh_acceptor]}],
 		supervisor:which_children(AccSup)),
     
-    ?wait_match([{_, _, worker,[ssh_connection_handler]}],
+    ?wait_match([{_, _, worker,[gen_statem]}],
 		supervisor:which_children(ConnectionSup)),
     
     ?wait_match([], supervisor:which_children(ChannelSup)),
@@ -433,7 +433,7 @@ check_sshc_system_tree(SysSup, Connection, LocalIP, LocalPort, _Config) ->
 		  [ssh_channel_sup]}],
 		supervisor:which_children(SubSysSup),
 		[ConnectionSup,ChannelSup,FwdAccSup]),
-    ?wait_match([{_, Connection, worker,[ssh_connection_handler]}],
+    ?wait_match([{_, Connection, worker,[gen_statem]}],
 		supervisor:which_children(ConnectionSup)),
     ?wait_match([], supervisor:which_children(ChannelSup)),
     ?wait_match([], supervisor:which_children(FwdAccSup)),
-- 
2.26.2

Places

File 3822-ssh-Refactor-for-changes-later.patch of Package erlang

Places