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
