From 1aeb8f4234b52705f9a933abf8dcd1afb2296b9d Mon Sep 17 00:00:00 2001 From: Ingela Anderton Andin Date: Wed, 22 Feb 2012 12:07:11 +0100 Subject: Prevent client hanging. (OTP-8111) Restored supervisor tree so that error propagation will work as intended, although connection processes are set to temporary, instead of permanent with restart times set to 0, and termination of the connection subtree is initiated by a temporary process spawned by ssh_connection_managers terminate. This is done to avoid unwanted supervisor reports. Pherhaps we need some new supervisor functionality. --- lib/ssh/src/Makefile | 2 +- lib/ssh/src/ssh.app.src | 2 +- lib/ssh/src/ssh.erl | 2 +- lib/ssh/src/ssh_acceptor.erl | 8 +- lib/ssh/src/ssh_connect.hrl | 6 +- lib/ssh/src/ssh_connection.erl | 6 +- lib/ssh/src/ssh_connection_handler.erl | 104 ++++++++++------------- lib/ssh/src/ssh_connection_manager.erl | 145 ++++++++++++++------------------- lib/ssh/src/ssh_connection_sup.erl | 113 +++++++++++++++++++++++++ lib/ssh/src/ssh_subsystem_sup.erl | 14 ++-- lib/ssh/src/ssh_system_sup.erl | 2 +- lib/ssh/src/ssh_transport.erl | 5 +- lib/ssh/src/sshc_sup.erl | 26 +++--- lib/ssh/test/ssh_basic_SUITE.erl | 67 +++++++++++---- lib/ssh/test/ssh_sftpd_SUITE.erl | 7 +- lib/ssh/test/ssh_test_lib.erl | 10 +++ 16 files changed, 321 insertions(+), 198 deletions(-) create mode 100644 lib/ssh/src/ssh_connection_sup.erl diff --git a/lib/ssh/src/Makefile b/lib/ssh/src/Makefile index 7be97abf66..1734ae4df4 100644 --- a/lib/ssh/src/Makefile +++ b/lib/ssh/src/Makefile @@ -45,11 +45,11 @@ MODULES= \ ssh_sup \ sshc_sup \ sshd_sup \ + ssh_connection_sup \ ssh_channel \ ssh_connection \ ssh_connection_handler \ ssh_connection_manager \ - ssh_connection_controler \ ssh_shell \ ssh_system_sup \ ssh_subsystem_sup \ diff --git a/lib/ssh/src/ssh.app.src b/lib/ssh/src/ssh.app.src index 7a58dbe54f..316c09eb06 100644 --- a/lib/ssh/src/ssh.app.src +++ b/lib/ssh/src/ssh.app.src @@ -15,7 +15,7 @@ ssh_connection, ssh_connection_handler, ssh_connection_manager, - ssh_connection_controler, + ssh_connection_sup, ssh_shell, sshc_sup, sshd_sup, diff --git a/lib/ssh/src/ssh.erl b/lib/ssh/src/ssh.erl index 3601bade8f..39c7fe329e 100644 --- a/lib/ssh/src/ssh.erl +++ b/lib/ssh/src/ssh.erl @@ -90,7 +90,7 @@ do_connect(Host, Port, SocketOptions, SshOptions, Timeout, DisableIpv6) -> {ssh_opts, SshOptions}]]) of {ok, ConnectionSup} -> {ok, Manager} = - ssh_connection_controler:connection_manager(ConnectionSup), + ssh_connection_sup:connection_manager(ConnectionSup), MRef = erlang:monitor(process, Manager), receive {Manager, is_connected} -> diff --git a/lib/ssh/src/ssh_acceptor.erl b/lib/ssh/src/ssh_acceptor.erl index 59fbd24cf5..d023656c32 100644 --- a/lib/ssh/src/ssh_acceptor.erl +++ b/lib/ssh/src/ssh_acceptor.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2008-2011. All Rights Reserved. +%% Copyright Ericsson AB 2008-2012. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -86,11 +86,11 @@ handle_connection(Callback, Address, Port, Options, Socket) -> {ok, SubSysSup} = ssh_system_sup:start_subsystem(SystemSup, Options), ConnectionSup = ssh_system_sup:connection_supervisor(SystemSup), {ok, Pid} = - ssh_connection_controler:start_manager_child(ConnectionSup, - [server, Socket, Options, SubSysSup]), + ssh_connection_sup:start_manager_child(ConnectionSup, + [server, Socket, Options]), Callback:controlling_process(Socket, Pid), SshOpts = proplists:get_value(ssh_opts, Options), - Pid ! {start_connection, server, [Address, Port, Socket, SshOpts]}. + Pid ! {start_connection, server, [Address, Port, Socket, SshOpts, SubSysSup]}. handle_error(timeout) -> ok; diff --git a/lib/ssh/src/ssh_connect.hrl b/lib/ssh/src/ssh_connect.hrl index e06c9ea211..932b0642f1 100644 --- a/lib/ssh/src/ssh_connect.hrl +++ b/lib/ssh/src/ssh_connect.hrl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2005-2011. All Rights Reserved. +%% Copyright Ericsson AB 2005-2012. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -253,7 +253,6 @@ -record(connection, { requests = [], %% [{ChannelId, Pid}...] awaiting reply on request, channel_cache, - channel_pids = [], port_bindings, channel_id_seed, cli_spec, @@ -261,5 +260,6 @@ port, options, exec, - sub_system_supervisor + sub_system_supervisor, + connection_supervisor }). diff --git a/lib/ssh/src/ssh_connection.erl b/lib/ssh/src/ssh_connection.erl index cb02d7b824..46f0c7e688 100644 --- a/lib/ssh/src/ssh_connection.erl +++ b/lib/ssh/src/ssh_connection.erl @@ -744,8 +744,8 @@ handle_msg(#ssh_msg_global_request{name = _Type, %%% This transport message will also be handled at the connection level handle_msg(#ssh_msg_disconnect{code = Code, - description = Description, - language = _Lang }, + description = Description, + language = _Lang }, #connection{channel_cache = Cache} = Connection0, _, _) -> {Connection, Replies} = ssh_channel:cache_foldl(fun(Channel, {Connection1, Acc}) -> @@ -779,7 +779,7 @@ handle_cli_msg(#connection{channel_cache = Cache} = Connection0, handle_cli_msg(Connection0, _, Channel, Reply0) -> {Reply, Connection} = reply_msg(Channel, Connection0, Reply0), {{replies, [Reply]}, Connection}. - + channel_eof_msg(ChannelId) -> #ssh_msg_channel_eof{recipient_channel = ChannelId}. diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index b3319fc809..5b3d1b8a1b 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -106,6 +106,7 @@ peer_address(ConnectionHandler) -> %% initialize. %%-------------------------------------------------------------------- init([Role, Manager, Socket, SshOpts]) -> + process_flag(trap_exit, true), {NumVsn, StrVsn} = ssh_transport:versions(Role, SshOpts), ssh_bits:install_messages(ssh_transport:transport_messages(NumVsn)), {Protocol, Callback, CloseTag} = @@ -283,8 +284,7 @@ new_keys(#ssh_msg_newkeys{} = Msg, #state{ssh_params = Ssh0} = State0) -> {next_state, NextStateName, next_packet(State)} catch #ssh_msg_disconnect{} = DisconnectMsg -> - handle_disconnect(DisconnectMsg, State0), - {stop, normal, State0}; + handle_disconnect(DisconnectMsg, State0); _:Error -> Desc = log_error(Error), handle_disconnect(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_KEY_EXCHANGE_FAILED, @@ -426,24 +426,11 @@ userauth(#ssh_msg_userauth_failure{authentications = Methodes}, %% The prefered authentication method failed try next method userauth(#ssh_msg_userauth_failure{}, - #state{ssh_params = #ssh{role = client} = Ssh0, - manager = Pid} = State) -> + #state{ssh_params = #ssh{role = client} = Ssh0} = State) -> case ssh_auth:userauth_request_msg(Ssh0) of - {disconnect, Event, {Msg, _}} -> - try - send_msg(Msg, State), - ssh_connection_manager:event(Pid, Event) - catch - exit:{noproc, _Reason} -> - Report = io_lib:format("Connection Manager terminated: ~p~n", - [Pid]), - error_logger:info_report(Report); - exit:Exit -> - Report = io_lib:format("Connection Manager returned:~n~p~n~p~n", - [Msg, Exit]), - error_logger:info_report(Report) - end, - {stop, normal, State}; + {disconnect, DisconnectMsg,{Msg, Ssh}} -> + send_msg(Msg, State), + handle_disconnect(DisconnectMsg, State#state{ssh_params = Ssh}); {Msg, Ssh} -> send_msg(Msg, State), {next_state, userauth, next_packet(State#state{ssh_params = Ssh})} @@ -614,7 +601,16 @@ handle_info({Protocol, Socket, Data}, Statename, handle_info({CloseTag, _Socket}, _StateName, #state{transport_close_tag = CloseTag, ssh_params = #ssh{role = _Role, opts = _Opts}} = State) -> - {stop, {shutdown, connection_lost}, State}; + DisconnectMsg = + #ssh_msg_disconnect{code = ?SSH_DISCONNECT_CONNECTION_LOST, + description = "Connection Lost", + language = "en"}, + {stop, {shutdown, DisconnectMsg}, State}; + +%%% So that terminate will be run when supervisor is shutdown +handle_info({'EXIT', _Sup, Reason}, _StateName, State) -> + {stop, Reason, State}; + handle_info(UnexpectedMessage, StateName, #state{ssh_params = SshParams} = State) -> Msg = lists:flatten(io_lib:format( "Unexpected message '~p' received in state '~p'\n" @@ -626,7 +622,6 @@ handle_info(UnexpectedMessage, StateName, #state{ssh_params = SshParams} = State error_logger:info_report(Msg), {next_state, StateName, State}. - %%-------------------------------------------------------------------- %% Function: terminate(Reason, StateName, State) -> void() %% Description:This function is called by a gen_fsm when it is about @@ -641,28 +636,31 @@ terminate(normal, _, #state{transport_cb = Transport, (catch Transport:close(Socket)), ok; -terminate(shutdown, _, State) -> +%% Terminated as manager terminated +terminate(shutdown, StateName, #state{ssh_params = Ssh0} = State) -> DisconnectMsg = #ssh_msg_disconnect{code = ?SSH_DISCONNECT_BY_APPLICATION, - description = "Application disconnect", + description = "Application shutdown", language = "en"}, - handle_disconnect(DisconnectMsg, State); - + {SshPacket, Ssh} = ssh_transport:ssh_packet(DisconnectMsg, Ssh0), + send_msg(SshPacket, State), + terminate(normal, StateName, State#state{ssh_params = Ssh}); -terminate({shutdown, connection_lost}, _, State) -> +terminate({shutdown, #ssh_msg_disconnect{} = Msg}, StateName, #state{ssh_params = Ssh0, manager = Pid} = State) -> + {SshPacket, Ssh} = ssh_transport:ssh_packet(Msg, Ssh0), + send_msg(SshPacket, State), + ssh_connection_manager:event(Pid, Msg), + terminate(normal, StateName, State#state{ssh_params = Ssh}); +terminate(Reason, StateName, #state{ssh_params = Ssh0, manager = Pid} = State) -> + log_error(Reason), DisconnectMsg = - #ssh_msg_disconnect{code = ?SSH_DISCONNECT_CONNECTION_LOST, - description = "Connection Lost", - language = "en"}, - handle_disconnect(DisconnectMsg, State); - -terminate(Reason, _, State) -> - Desc = log_error(Reason), - DisconnectMsg = - #ssh_msg_disconnect{code = ?SSH_DISCONNECT_SERVICE_NOT_AVAILABLE, - description = Desc, + #ssh_msg_disconnect{code = ?SSH_DISCONNECT_BY_APPLICATION, + description = "Internal error", language = "en"}, - handle_disconnect(DisconnectMsg, State). + {SshPacket, Ssh} = ssh_transport:ssh_packet(DisconnectMsg, Ssh0), + ssh_connection_manager:event(Pid, DisconnectMsg), + send_msg(SshPacket, State), + terminate(normal, StateName, State#state{ssh_params = Ssh}). %%-------------------------------------------------------------------- %% Function: @@ -808,11 +806,8 @@ generate_event(<> = Msg, StateName, next_packet(State), {next_state, StateName, State} catch - exit:{noproc, _Reason} -> - Report = io_lib:format("~p Connection Handler terminated: ~p~n", - [self(), Pid]), - error_logger:info_report(Report), - {stop, normal, State0} + exit:{noproc, Reason} -> + {stop, {shutdown, Reason}, State0} end; generate_event(Msg, StateName, State0, EncData) -> Event = ssh_bits:decode(Msg), @@ -909,24 +904,8 @@ handle_ssh_packet(Length, StateName, #state{decoded_data_buffer = DecData0, handle_disconnect(DisconnectMsg, State0) end. -handle_disconnect(#ssh_msg_disconnect{} = Msg, - #state{ssh_params = Ssh0, manager = Pid} = State) -> - {SshPacket, Ssh} = ssh_transport:ssh_packet(Msg, Ssh0), - try - send_msg(SshPacket, State), - ssh_connection_manager:event(Pid, Msg) - catch - exit:{noproc, _Reason} -> - Report = io_lib:format("~p Connection Manager terminated: ~p~n", - [self(), Pid]), - error_logger:info_report(Report); - exit:Exit -> - Report = io_lib:format("Connection Manager returned:~n~p~n~p~n", - [Msg, Exit]), - error_logger:info_report(Report) - end, - (catch ssh_userreg:delete_user(Pid)), - {stop, normal, State#state{ssh_params = Ssh}}. +handle_disconnect(#ssh_msg_disconnect{} = Msg, State) -> + {stop, {shutdown, Msg}, State}. counterpart_versions(NumVsn, StrVsn, #ssh{role = server} = Ssh) -> Ssh#ssh{c_vsn = NumVsn , c_version = StrVsn}; @@ -987,7 +966,8 @@ ssh_info([ _ | Rest], SshParams, Acc) -> log_error(Reason) -> Report = io_lib:format("Erlang ssh connection handler failed with reason: " - "~p , please report this to erlang-bugs@erlang.org \n", - [Reason]), + "~p ~n, Stacktace: ~p ~n" + "please report this to erlang-bugs@erlang.org \n", + [Reason, erlang:get_stacktrace()]), error_logger:error_report(Report), "Internal error". diff --git a/lib/ssh/src/ssh_connection_manager.erl b/lib/ssh/src/ssh_connection_manager.erl index f729276e65..406a042d72 100644 --- a/lib/ssh/src/ssh_connection_manager.erl +++ b/lib/ssh/src/ssh_connection_manager.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2008-2011. All Rights Reserved. +%% Copyright Ericsson AB 2008-2012. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -182,17 +182,15 @@ send_eof(ConnectionManager, ChannelId) -> %% {stop, Reason} %% Description: Initiates the server %%-------------------------------------------------------------------- -init([server, _Socket, Opts, SubSysSup]) -> +init([server, _Socket, Opts]) -> process_flag(trap_exit, true), ssh_bits:install_messages(ssh_connection:messages()), - Cache = ssh_channel:cache_create(), + Cache = ssh_channel:cache_create(), {ok, #state{role = server, connection_state = #connection{channel_cache = Cache, channel_id_seed = 0, port_bindings = [], - requests = [], - channel_pids = [], - sub_system_supervisor = SubSysSup}, + requests = []}, opts = Opts, connected = false}}; @@ -207,15 +205,14 @@ init([client, Opts]) -> Options = proplists:get_value(ssh_opts, Opts), ChannelPid = proplists:get_value(channel_pid, Opts), self() ! - {start_connection, client, [Parent, Address, Port, - ChannelPid, SocketOpts, Options]}, + {start_connection, client, [Parent, Address, Port, SocketOpts, Options]}, {ok, #state{role = client, client = ChannelPid, connection_state = #connection{channel_cache = Cache, channel_id_seed = 0, port_bindings = [], - requests = [], - channel_pids = []}, + connection_supervisor = Parent, + requests = []}, opts = Opts, connected = false}}. @@ -269,29 +266,25 @@ handle_call({ssh_msg, Pid, Msg}, From, when Role == client andalso (not IsConnected) -> lists:foreach(fun send_msg/1, Replies), ClientPid ! {self(), not_connected, Reason}, - {stop, normal, State#state{connection = Connection}}; + {stop, {shutdown, normal}, State#state{connection = Connection}}; {disconnect, Reason, {{replies, Replies}, Connection}} -> lists:foreach(fun send_msg/1, Replies), SSHOpts = proplists:get_value(ssh_opts, Opts), disconnect_fun(Reason, SSHOpts), - {stop, normal, State#state{connection_state = Connection}} + {stop, {shutdown, normal}, State#state{connection_state = Connection}} catch - exit:{noproc, Reason} -> - Report = io_lib:format("Connection probably terminated:~n~p~n~p~n~p~n", - [ConnectionMsg, Reason, erlang:get_stacktrace()]), - error_logger:info_report(Report), - {noreply, State}; - error:Error -> - Report = io_lib:format("Connection message returned:~n~p~n~p~n~p~n", - [ConnectionMsg, Error, erlang:get_stacktrace()]), - error_logger:info_report(Report), - {noreply, State}; - exit:Exit -> - Report = io_lib:format("Connection message returned:~n~p~n~p~n~p~n", - [ConnectionMsg, Exit, erlang:get_stacktrace()]), - error_logger:info_report(Report), - {noreply, State} - end; + _:Reason -> + {disconnect, Reason, {{replies, Replies}, Connection}} = + ssh_connection:handle_msg( + #ssh_msg_disconnect{code = ?SSH_DISCONNECT_BY_APPLICATION, + description = "Internal error", + language = "en"}, Connection0, undefined, + Role), + lists:foreach(fun send_msg/1, Replies), + SSHOpts = proplists:get_value(ssh_opts, Opts), + disconnect_fun(Reason, SSHOpts), + {stop, {shutdown, Reason}, State#state{connection_state = Connection}} + end; handle_call({global_request, Pid, _, _, _} = Request, From, #state{connection_state = @@ -341,7 +334,8 @@ handle_call({info, ChannelPid}, _From, handle_call({open, ChannelPid, Type, InitialWindowSize, MaxPacketSize, Data}, From, #state{connection = Pid, connection_state = - #connection{channel_cache = Cache}} = State0) -> + #connection{channel_cache = Cache}} = State0) -> + erlang:monitor(process, ChannelPid), {ChannelId, State1} = new_channel_id(State0), Msg = ssh_connection:channel_open_msg(Type, ChannelId, InitialWindowSize, @@ -404,18 +398,18 @@ handle_call({close, ChannelId}, _, {reply, ok, State} end; -handle_call(stop, _, #state{role = _client, - client = _ChannelPid, - connection = Pid} = State) -> - DisconnectMsg = - #ssh_msg_disconnect{code = ?SSH_DISCONNECT_BY_APPLICATION, - description = "Application disconnect", - language = "en"}, - (catch gen_fsm:send_all_state_event(Pid, DisconnectMsg)), -% ssh_connection_handler:send(Pid, DisconnectMsg), - {stop, normal, ok, State}; -handle_call(stop, _, State) -> - {stop, normal, ok, State}; +handle_call(stop, _, #state{connection_state = Connection0, + role = Role, + opts = Opts} = State) -> + {disconnect, Reason, {{replies, Replies}, Connection}} = + ssh_connection:handle_msg(#ssh_msg_disconnect{code = ?SSH_DISCONNECT_BY_APPLICATION, + description = "User closed down connection", + language = "en"}, Connection0, undefined, + Role), + lists:foreach(fun send_msg/1, Replies), + SSHOpts = proplists:get_value(ssh_opts, Opts), + disconnect_fun(Reason, SSHOpts), + {stop, normal, ok, State#state{connection_state = Connection}}; %% API violation make it the violaters problem %% by ignoring it. The violating process will get @@ -493,31 +487,33 @@ handle_cast({failure, ChannelId}, #state{connection = Pid} = State) -> %% Description: Handling all non call/cast messages %%-------------------------------------------------------------------- handle_info({start_connection, server, - [Address, Port, Socket, Options]}, + [Address, Port, Socket, Options, SubSysSup]}, #state{connection_state = CState} = State) -> {ok, Connection} = ssh_transport:accept(Address, Port, Socket, Options), Shell = proplists:get_value(shell, Options), Exec = proplists:get_value(exec, Options), CliSpec = proplists:get_value(ssh_cli, Options, {ssh_cli, [Shell]}), + ssh_connection_handler:send_event(Connection, socket_control), {noreply, State#state{connection = Connection, connection_state = CState#connection{address = Address, port = Port, cli_spec = CliSpec, options = Options, - exec = Exec}}}; + exec = Exec, + sub_system_supervisor = SubSysSup + }}}; handle_info({start_connection, client, - [Parent, Address, Port, ChannelPid, SocketOpts, Options]}, - State) -> + [Parent, Address, Port, SocketOpts, Options]}, + #state{client = Pid} = State) -> case (catch ssh_transport:connect(Parent, Address, Port, SocketOpts, Options)) of {ok, Connection} -> - erlang:monitor(process, ChannelPid), {noreply, State#state{connection = Connection}}; Reason -> - ChannelPid ! {self(), not_connected, Reason}, - {stop, normal, State} + Pid ! {self(), not_connected, Reason}, + {stop, {shutdown, normal}, State} end; handle_info({ssh_cm, _Sender, Msg}, State0) -> @@ -537,20 +533,13 @@ handle_info(ssh_connected, #state{role = client, client = Pid} handle_info(ssh_connected, #state{role = server} = State) -> {noreply, State#state{connected = true}}; -handle_info({'DOWN', _Ref, process, ChannelPid, normal}, State0) -> - handle_down(handle_channel_down(ChannelPid, State0)); - -handle_info({'DOWN', _Ref, process, ChannelPid, shutdown}, State0) -> - handle_down(handle_channel_down(ChannelPid, State0)); - -handle_info({'DOWN', _Ref, process, ChannelPid, Reason}, State0) -> - Report = io_lib:format("Pid ~p DOWN ~p\n", [ChannelPid, Reason]), - error_logger:error_report(Report), - handle_down(handle_channel_down(ChannelPid, State0)); +%%% Handle that ssh channels user process goes down +handle_info({'DOWN', _Ref, process, ChannelPid, _Reason}, State) -> + handle_down(handle_channel_down(ChannelPid, State)); -handle_info({'EXIT', _, _}, State) -> - %% Handled in 'DOWN' - {noreply, State}. +%%% So that terminate will be run when supervisor is shutdown +handle_info({'EXIT', _Sup, Reason}, State) -> + {stop, Reason, State}. %%-------------------------------------------------------------------- %% Function: terminate(Reason, State) -> void() @@ -559,20 +548,19 @@ handle_info({'EXIT', _, _}, State) -> %% cleaning up. When it returns, the gen_server terminates with Reason. %% The return value is ignored. %%-------------------------------------------------------------------- -terminate(Reason, #state{connection_state = - #connection{requests = Requests, - sub_system_supervisor = SubSysSup}, +terminate(_Reason, #state{role = client, + connection_state = + #connection{connection_supervisor = Supervisor}}) -> + sshc_sup:stop_child(Supervisor); + +terminate(_Reason, #state{role = server, + connection_state = + #connection{sub_system_supervisor = SubSysSup}, opts = Opts}) -> - SSHOpts = proplists:get_value(ssh_opts, Opts), - disconnect_fun(Reason, SSHOpts), - (catch lists:foreach(fun({_, From}) -> - gen_server:reply(From, {error, connection_closed}) - end, Requests)), Address = proplists:get_value(address, Opts), Port = proplists:get_value(port, Opts), SystemSup = ssh_system_sup:system_supervisor(Address, Port), - ssh_system_sup:stop_subsystem(SystemSup, SubSysSup), - ok. + ssh_system_sup:stop_subsystem(SystemSup, SubSysSup). %%-------------------------------------------------------------------- %% Func: code_change(OldVsn, State, Extra) -> {ok, NewState} @@ -619,16 +607,7 @@ decode_ssh_msg(Msg) -> send_msg(Msg) -> - case catch do_send_msg(Msg) of - {'EXIT', Reason}-> - Report = io_lib:format("Connection Manager fail to send:~n~p~n" - "Reason why it failed was:~n~p~n", - [Msg, Reason]), - error_logger:info_report(Report); - _ -> - ok - end. - + catch do_send_msg(Msg). do_send_msg({channel_data, Pid, Data}) -> Pid ! {ssh_cm, self(), Data}; do_send_msg({channel_requst_reply, From, Data}) -> @@ -676,8 +655,8 @@ handle_down({{replies, Replies}, State}) -> {noreply, State}. handle_channel_down(ChannelPid, #state{connection_state = - #connection{channel_cache = Cache}} = - State) -> + #connection{channel_cache = Cache}} = + State) -> ssh_channel:cache_foldl( fun(Channel, Acc) when Channel#channel.user == ChannelPid -> ssh_channel:cache_delete(Cache, diff --git a/lib/ssh/src/ssh_connection_sup.erl b/lib/ssh/src/ssh_connection_sup.erl new file mode 100644 index 0000000000..e3544af1c6 --- /dev/null +++ b/lib/ssh/src/ssh_connection_sup.erl @@ -0,0 +1,113 @@ +%% +%% %CopyrightBegin% +%% +%% Copyright Ericsson AB 2008-2012. All Rights Reserved. +%% +%% The contents of this file are subject to the Erlang Public License, +%% Version 1.1, (the "License"); you may not use this file except in +%% compliance with the License. You should have received a copy of the +%% Erlang Public License along with this software. If not, it can be +%% retrieved online at http://www.erlang.org/. +%% +%% Software distributed under the License is distributed on an "AS IS" +%% basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See +%% the License for the specific language governing rights and limitations +%% under the License. +%% +%% %CopyrightEnd% +%% +%% +%%---------------------------------------------------------------------- +%% Purpose: Ssh connection supervisor. +%%---------------------------------------------------------------------- + +-module(ssh_connection_sup). + +-behaviour(supervisor). + +-export([start_link/1, start_handler_child/2, start_manager_child/2, + connection_manager/1]). + +%% Supervisor callback +-export([init/1]). + +%%%========================================================================= +%%% API +%%%========================================================================= +start_link(Args) -> + supervisor:start_link(?MODULE, [Args]). + +%% Will be called from the manager child process +start_handler_child(Sup, Args) -> + [Spec] = child_specs(handler, Args), + supervisor:start_child(Sup, Spec). + +%% Will be called from the acceptor process +start_manager_child(Sup, Args) -> + [Spec] = child_specs(manager, Args), + supervisor:start_child(Sup, Spec). + +connection_manager(SupPid) -> + Children = supervisor:which_children(SupPid), + {ok, ssh_connection_manager(Children)}. + +%%%========================================================================= +%%% Supervisor callback +%%%========================================================================= +init([Args]) -> + RestartStrategy = one_for_all, + MaxR = 0, + MaxT = 3600, + Children = child_specs(Args), + {ok, {{RestartStrategy, MaxR, MaxT}, Children}}. + +%%%========================================================================= +%%% Internal functions +%%%========================================================================= +child_specs(Opts) -> + case proplists:get_value(role, Opts) of + client -> + child_specs(manager, [client | Opts]); + server -> + %% Children started by acceptor process + [] + end. + +% The manager process starts the handler process +child_specs(manager, Opts) -> + [manager_spec(Opts)]; +child_specs(handler, Opts) -> + [handler_spec(Opts)]. + +manager_spec([server = Role, Socket, Opts]) -> + Name = make_ref(), + StartFunc = {ssh_connection_manager, start_link, [[Role, Socket, Opts]]}, + Restart = temporary, + Shutdown = 3600, + Modules = [ssh_connection_manager], + Type = worker, + {Name, StartFunc, Restart, Shutdown, Type, Modules}; + +manager_spec([client = Role | Opts]) -> + Name = make_ref(), + StartFunc = {ssh_connection_manager, start_link, [[Role, Opts]]}, + Restart = temporary, + Shutdown = 3600, + Modules = [ssh_connection_manager], + Type = worker, + {Name, StartFunc, Restart, Shutdown, Type, Modules}. + +handler_spec([Role, Socket, Opts]) -> + Name = make_ref(), + StartFunc = {ssh_connection_handler, + start_link, [Role, self(), Socket, Opts]}, + Restart = temporary, + Shutdown = 3600, + Modules = [ssh_connection_handler], + Type = worker, + {Name, StartFunc, Restart, Shutdown, Type, Modules}. + +ssh_connection_manager([{_, Child, _, [ssh_connection_manager]} | _]) -> + Child; +ssh_connection_manager([_ | Rest]) -> + ssh_connection_manager(Rest). diff --git a/lib/ssh/src/ssh_subsystem_sup.erl b/lib/ssh/src/ssh_subsystem_sup.erl index d71b6bbc56..cd6defd535 100644 --- a/lib/ssh/src/ssh_subsystem_sup.erl +++ b/lib/ssh/src/ssh_subsystem_sup.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2008-2010. All Rights Reserved. +%% Copyright Ericsson AB 2008-2012. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -70,13 +70,12 @@ ssh_connectinon_child_spec(Opts) -> Address = proplists:get_value(address, Opts), Port = proplists:get_value(port, Opts), Role = proplists:get_value(role, Opts), - Name = id(Role, ssh_connection_controler, Address, Port), - StartFunc = {ssh_connection_controler, start_link, [Opts]}, + Name = id(Role, ssh_connection_sup, Address, Port), + StartFunc = {ssh_connection_sup, start_link, [Opts]}, Restart = transient, -% Restart = permanent, Shutdown = 5000, - Modules = [ssh_connection_controler], - Type = worker, + Modules = [ssh_connection_sup], + Type = supervisor, {Name, StartFunc, Restart, Shutdown, Type, Modules}. ssh_channel_child_spec(Opts) -> @@ -86,7 +85,6 @@ ssh_channel_child_spec(Opts) -> Name = id(Role, ssh_channel_sup, Address, Port), StartFunc = {ssh_channel_sup, start_link, [Opts]}, Restart = transient, -% Restart = permanent, Shutdown = infinity, Modules = [ssh_channel_sup], Type = supervisor, @@ -95,7 +93,7 @@ ssh_channel_child_spec(Opts) -> id(Role, Sup, Address, Port) -> {Role, Sup, Address, Port}. -ssh_connection_sup([{_, Child, _, [ssh_connection_controler]} | _]) -> +ssh_connection_sup([{_, Child, _, [ssh_connection_sup]} | _]) -> Child; ssh_connection_sup([_ | Rest]) -> ssh_connection_sup(Rest). diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl index 920baaadef..23480f5fe3 100644 --- a/lib/ssh/src/ssh_system_sup.erl +++ b/lib/ssh/src/ssh_system_sup.erl @@ -146,7 +146,7 @@ ssh_acceptor_child_spec(ServerOpts) -> ssh_subsystem_child_spec(ServerOpts) -> Name = make_ref(), StartFunc = {ssh_subsystem_sup, start_link, [ServerOpts]}, - Restart = temporary, + Restart = transient, Shutdown = infinity, Modules = [ssh_subsystem_sup], Type = supervisor, diff --git a/lib/ssh/src/ssh_transport.erl b/lib/ssh/src/ssh_transport.erl index 6140c87f6e..1f912c9bdf 100644 --- a/lib/ssh/src/ssh_transport.erl +++ b/lib/ssh/src/ssh_transport.erl @@ -142,7 +142,7 @@ connect(ConnectionSup, Address, Port, SocketOpts, Opts) -> case do_connect(Callback, Address, Port, SocketOpts, Timeout) of {ok, Socket} -> {ok, Pid} = - ssh_connection_controler:start_handler_child(ConnectionSup, + ssh_connection_sup:start_handler_child(ConnectionSup, [client, Socket, [{address, Address}, {port, Port} | @@ -174,12 +174,11 @@ accept(Address, Port, Socket, Options) -> ssh_system_sup:connection_supervisor( ssh_system_sup:system_supervisor(Address, Port)), {ok, Pid} = - ssh_connection_controler:start_handler_child(ConnectionSup, + ssh_connection_sup:start_handler_child(ConnectionSup, [server, Socket, [{address, Address}, {port, Port} | Options]]), Callback:controlling_process(Socket, Pid), - ssh_connection_handler:send_event(Pid, socket_control), {ok, Pid}. format_version({Major,Minor}) -> diff --git a/lib/ssh/src/sshc_sup.erl b/lib/ssh/src/sshc_sup.erl index 7c29c669e4..1d2779de23 100644 --- a/lib/ssh/src/sshc_sup.erl +++ b/lib/ssh/src/sshc_sup.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2008-2010. All Rights Reserved. +%% Copyright Ericsson AB 2008-2012. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -26,7 +26,7 @@ -behaviour(supervisor). --export([start_link/1, start_child/1]). +-export([start_link/1, start_child/1, stop_child/1]). %% Supervisor callback -export([init/1]). @@ -40,12 +40,19 @@ start_link(Args) -> start_child(Args) -> supervisor:start_child(?MODULE, Args). +stop_child(Client) -> + spawn(fun() -> + ClientSup = whereis(?MODULE), + supervisor:terminate_child(ClientSup, Client) + end), + ok. + %%%========================================================================= %%% Supervisor callback %%%========================================================================= init(Args) -> RestartStrategy = simple_one_for_one, - MaxR = 10, + MaxR = 0, MaxT = 3600, {ok, {{RestartStrategy, MaxR, MaxT}, [child_spec(Args)]}}. @@ -54,12 +61,9 @@ init(Args) -> %%%========================================================================= child_spec(_) -> Name = undefined, % As simple_one_for_one is used. - StartFunc = {ssh_connection_controler, start_link, []}, - Restart = temporary, -% Shutdown = infinity, - Shutdown = 5000, - Modules = [ssh_connection_controler], -% Type = supervisor, - Type = worker, + StartFunc = {ssh_connection_sup, start_link, []}, + Restart = temporary, + Shutdown = infinity, + Modules = [ssh_connection_sup], + Type = supervisor, {Name, StartFunc, Restart, Shutdown, Type, Modules}. - diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl index 8ec037dd8f..012367a6df 100644 --- a/lib/ssh/test/ssh_basic_SUITE.erl +++ b/lib/ssh/test/ssh_basic_SUITE.erl @@ -88,7 +88,7 @@ end_per_testcase(TestCase, Config) when TestCase == server_password_option; UserDir = filename:join(?config(priv_dir, Config), nopubkey), ssh_test_lib:del_dirs(UserDir), end_per_testcase(Config); -end_per_testcase(TestCase, Config) -> +end_per_testcase(_TestCase, Config) -> end_per_testcase(Config). end_per_testcase(_Config) -> ssh:stop(), @@ -108,14 +108,16 @@ all() -> {group, rsa_key}, {group, dsa_pass_key}, {group, rsa_pass_key}, + {group, internal_error}, daemon_already_started, server_password_option, server_userpassword_option]. groups() -> [{dsa_key, [], [exec, exec_compressed, shell, known_hosts]}, - {rsa_key, [], [exec, exec_compressed, shell, known_hosts]}, + {rsa_key, [], [exec, exec_compressed, shell, known_hosts]}, {dsa_pass_key, [], [pass_phrase]}, - {rsa_pass_key, [], [pass_phrase]} + {rsa_pass_key, [], [pass_phrase]}, + {internal_error, [], [internal_error]} ]. init_per_group(dsa_key, Config) -> @@ -138,6 +140,12 @@ init_per_group(dsa_pass_key, Config) -> PrivDir = ?config(priv_dir, Config), ssh_test_lib:setup_dsa_pass_pharse(DataDir, PrivDir, "Password"), [{pass_phrase, {dsa_pass_phrase, "Password"}}| Config]; +init_per_group(internal_error, Config) -> + DataDir = ?config(data_dir, Config), + PrivDir = ?config(priv_dir, Config), + ssh_test_lib:setup_dsa(DataDir, PrivDir), + file:delete(filename:join(PrivDir, "system/ssh_host_dsa_key")), + Config; init_per_group(_, Config) -> Config. @@ -157,6 +165,11 @@ end_per_group(rsa_pass_key, Config) -> PrivDir = ?config(priv_dir, Config), ssh_test_lib:clean_rsa(PrivDir), Config; +end_per_group(internal_error, Config) -> + PrivDir = ?config(priv_dir, Config), + ssh_test_lib:clean_dsa(PrivDir), + Config; + end_per_group(_, Config) -> Config. @@ -268,9 +281,14 @@ shell(Config) when is_list(Config) -> IO = ssh_test_lib:start_io_server(), Shell = ssh_test_lib:start_shell(Port, IO, UserDir), receive + {'EXIT', _, _} -> + test_server:fail(no_ssh_connection); ErlShellStart -> - test_server:format("Erlang shell start: ~p~n", [ErlShellStart]) - end, + test_server:format("Erlang shell start: ~p~n", [ErlShellStart]), + do_shell(IO, Shell) + end. + +do_shell(IO, Shell) -> receive ErlPrompt0 -> test_server:format("Erlang prompt: ~p~n", [ErlPrompt0]) @@ -361,6 +379,9 @@ server_password_option(Config) when is_list(Config) -> {password, "morot"}, {user_interaction, false}, {user_dir, UserDir}]), + + Reason = "Unable to connect using the available authentication methods", + {error, Reason} = ssh:connect(Host, Port, [{silently_accept_hosts, true}, {user, "vego"}, @@ -396,25 +417,20 @@ server_userpassword_option(Config) when is_list(Config) -> {user_dir, UserDir}]), ssh:close(ConnectionRef), - {error, Reason0} = + Reason = "Unable to connect using the available authentication methods", + + {error, Reason} = ssh:connect(Host, Port, [{silently_accept_hosts, true}, {user, "foo"}, {password, "morot"}, {user_interaction, false}, {user_dir, UserDir}]), - - test_server:format("Test of user foo that does not exist. " - "Error msg: ~p ~n", [Reason0]), - - {error, Reason1} = + {error, Reason} = ssh:connect(Host, Port, [{silently_accept_hosts, true}, {user, "vego"}, {password, "foo"}, {user_interaction, false}, {user_dir, UserDir}]), - test_server:format("Test of wrong Password. " - "Error msg: ~p ~n", [Reason1]), - ssh:stop_daemon(Pid). %%-------------------------------------------------------------------- @@ -445,6 +461,7 @@ known_hosts(Config) when is_list(Config) -> [Host, _Ip] = string:tokens(HostAndIp, ","), "ssh-" ++ _ = Alg, ssh:stop_daemon(Pid). +%%-------------------------------------------------------------------- pass_phrase(doc) -> ["Test that we can use keyes protected by pass phrases"]; @@ -469,6 +486,28 @@ pass_phrase(Config) when is_list(Config) -> {ok, _ChannelId} = ssh_connection:session_channel(ConnectionRef, infinity), ssh:stop_daemon(Pid). +%%-------------------------------------------------------------------- + +internal_error(doc) -> + ["Test that client does not hang if disconnects due to internal error"]; + +internal_error(suite) -> + []; + +internal_error(Config) when is_list(Config) -> + process_flag(trap_exit, true), + SystemDir = filename:join(?config(priv_dir, Config), system), + UserDir = ?config(priv_dir, Config), + + {_Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir}, + {user_dir, UserDir}, + {failfun, fun ssh_test_lib:failfun/2}]), + {error,"Internal error"} = + ssh:connect(Host, Port, [{silently_accept_hosts, true}, + {user_dir, UserDir}, + {user_interaction, false}]). + + %%-------------------------------------------------------------------- %% Internal functions %%-------------------------------------------------------------------- diff --git a/lib/ssh/test/ssh_sftpd_SUITE.erl b/lib/ssh/test/ssh_sftpd_SUITE.erl index 6e4480ee9d..de946d4c4c 100644 --- a/lib/ssh/test/ssh_sftpd_SUITE.erl +++ b/lib/ssh/test/ssh_sftpd_SUITE.erl @@ -30,7 +30,6 @@ -include_lib("kernel/include/file.hrl"). --define(SFPD_PORT, 9999). -define(USER, "Alladin"). -define(PASSWD, "Sesame"). -define(XFER_PACKET_SIZE, 32768). @@ -102,13 +101,15 @@ init_per_testcase(TestCase, Config) -> ClientUserDir = filename:join(PrivDir, nopubkey), SystemDir = filename:join(?config(priv_dir, Config), system), + Port = ssh_test_lib:inet_port(node()), + {ok, Sftpd} = - ssh_sftpd:listen(?SFPD_PORT, [{system_dir, SystemDir}, + ssh_sftpd:listen(Port, [{system_dir, SystemDir}, {user_dir, PrivDir}, {user_passwords,[{?USER, ?PASSWD}]}, {pwdfun, fun(_,_) -> true end}]), - Cm = ssh_test_lib:connect(?SFPD_PORT, + Cm = ssh_test_lib:connect(Port, [{user_dir, ClientUserDir}, {user, ?USER}, {password, ?PASSWD}, {user_interaction, false}, diff --git a/lib/ssh/test/ssh_test_lib.erl b/lib/ssh/test/ssh_test_lib.erl index c8a4fe1c62..609663c87a 100644 --- a/lib/ssh/test/ssh_test_lib.erl +++ b/lib/ssh/test/ssh_test_lib.erl @@ -334,3 +334,13 @@ del_dirs(Dir) -> _ -> ok end. + +inet_port(Node) -> + {Port, Socket} = do_inet_port(Node), + rpc:call(Node, gen_tcp, close, [Socket]), + Port. + +do_inet_port(Node) -> + {ok, Socket} = rpc:call(Node, gen_tcp, listen, [0, [{reuseaddr, true}]]), + {ok, Port} = rpc:call(Node, inet, port, [Socket]), + {Port, Socket}. -- cgit v1.2.3