aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorIngela Anderton Andin <[email protected]>2013-11-22 14:54:43 +0100
committerIngela Anderton Andin <[email protected]>2013-11-26 12:24:25 +0100
commite4653d52abd98628fb862a8b01ea804473bdb338 (patch)
treeaa4263814d11aeca186f07ac4fc9485afcb9fa6f
parent90c8450bb691a7b57ce326fddd1f44ef01db390f (diff)
downloadotp-e4653d52abd98628fb862a8b01ea804473bdb338.tar.gz
otp-e4653d52abd98628fb862a8b01ea804473bdb338.tar.bz2
otp-e4653d52abd98628fb862a8b01ea804473bdb338.zip
ssh: Correct close handling
Commit 68263a48bfbdac4dc219a91f06af3d535d881850 got close handling slightly wrong, channels did not get their close message. Commit 32102f1e8225dada7526c9bfee6622f9026ba4cd did not work as expected
-rw-r--r--lib/ssh/src/ssh_acceptor_sup.erl6
-rw-r--r--lib/ssh/src/ssh_connection.erl1
-rw-r--r--lib/ssh/src/ssh_connection_handler.erl27
-rw-r--r--lib/ssh/src/ssh_system_sup.erl15
-rw-r--r--lib/ssh/src/sshd_sup.erl11
-rw-r--r--lib/ssh/test/ssh_basic_SUITE.erl38
6 files changed, 67 insertions, 31 deletions
diff --git a/lib/ssh/src/ssh_acceptor_sup.erl b/lib/ssh/src/ssh_acceptor_sup.erl
index f37e1fe4ff..2be729d305 100644
--- a/lib/ssh/src/ssh_acceptor_sup.erl
+++ b/lib/ssh/src/ssh_acceptor_sup.erl
@@ -1,7 +1,7 @@
%%
%% %CopyrightBegin%
%%
-%% Copyright Ericsson AB 2008-2010. All Rights Reserved.
+%% Copyright Ericsson AB 2008-2013. 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
@@ -84,8 +84,8 @@ child_spec(ServerOpts) ->
[{active, false},
{reuseaddr, true}] ++ SocketOpts,
ServerOpts, Timeout]},
- Restart = permanent,
- Shutdown = 3600,
+ Restart = transient,
+ Shutdown = brutal_kill,
Modules = [ssh_acceptor],
Type = worker,
{Name, StartFunc, Restart, Shutdown, Type, Modules}.
diff --git a/lib/ssh/src/ssh_connection.erl b/lib/ssh/src/ssh_connection.erl
index 7016f349e8..03dddae3c8 100644
--- a/lib/ssh/src/ssh_connection.erl
+++ b/lib/ssh/src/ssh_connection.erl
@@ -730,7 +730,6 @@ handle_msg(#ssh_msg_request_success{data = Data},
{{replies, [{channel_requst_reply, From, {success, Data}}]},
Connection#connection{requests = Rest}};
-%%% This transport message will also be handled at the connection level
handle_msg(#ssh_msg_disconnect{code = Code,
description = Description,
language = _Lang },
diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl
index 7ba2179a76..3462b98172 100644
--- a/lib/ssh/src/ssh_connection_handler.erl
+++ b/lib/ssh/src/ssh_connection_handler.erl
@@ -519,7 +519,8 @@ connected({#ssh_msg_kexinit{}, _Payload} = Event, State) ->
#state{}) -> gen_fsm_state_return().
%%--------------------------------------------------------------------
-handle_event(#ssh_msg_disconnect{description = Desc}, _StateName, #state{} = State) ->
+handle_event(#ssh_msg_disconnect{description = Desc} = DisconnectMsg, _StateName, #state{} = State) ->
+ handle_disconnect(DisconnectMsg, State),
{stop, {shutdown, Desc}, State};
handle_event(#ssh_msg_ignore{}, StateName, State) ->
@@ -850,7 +851,11 @@ 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_BY_APPLICATION,
+ description = "Connection closed",
+ language = "en"},
+ handle_disconnect(DisconnectMsg, State);
handle_info({timeout, {_, From} = Request}, Statename,
#state{connection_state = #connection{requests = Requests} = Connection} = State) ->
@@ -1377,10 +1382,16 @@ handle_ssh_packet(Length, StateName, #state{decoded_data_buffer = DecData0,
handle_disconnect(DisconnectMsg, State0)
end.
-handle_disconnect(#ssh_msg_disconnect{description = Desc}, State) ->
- {stop, {shutdown, Desc}, State}.
-handle_disconnect(#ssh_msg_disconnect{description = Desc}, State, ErrorMsg) ->
- {stop, {shutdown, {Desc, ErrorMsg}}, State}.
+handle_disconnect(#ssh_msg_disconnect{description = Desc} = Msg, #state{connection_state = Connection0,
+ role = Role} = State0) ->
+ {disconnect, _, {{replies, Replies}, Connection}} = ssh_connection:handle_msg(Msg, Connection0, Role),
+ State = send_replies(Replies, State0),
+ {stop, {shutdown, Desc}, State#state{connection_state = Connection}}.
+handle_disconnect(#ssh_msg_disconnect{description = Desc} = Msg, #state{connection_state = Connection0,
+ role = Role} = State0, ErrorMsg) ->
+ {disconnect, _, {{replies, Replies}, Connection}} = ssh_connection:handle_msg(Msg, Connection0, Role),
+ State = send_replies(Replies, State0),
+ {stop, {shutdown, {Desc, ErrorMsg}}, State#state{connection_state = Connection}}.
counterpart_versions(NumVsn, StrVsn, #ssh{role = server} = Ssh) ->
Ssh#ssh{c_vsn = NumVsn , c_version = StrVsn};
@@ -1420,9 +1431,9 @@ retry_fun(User, PeerAddr, Reason, Opts) ->
do_retry_fun(Fun, User, PeerAddr, Reason) ->
case erlang:fun_info(Fun, arity) of
- 2 -> %% Backwards compatible
+ {arity, 2} -> %% Backwards compatible
catch Fun(User, Reason);
- 3 ->
+ {arity, 3} ->
catch Fun(User, PeerAddr, Reason)
end.
diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl
index bf3c12a988..848133f838 100644
--- a/lib/ssh/src/ssh_system_sup.erl
+++ b/lib/ssh/src/ssh_system_sup.erl
@@ -54,13 +54,16 @@ stop_listener(SysSup) ->
stop_listener(Address, Port) ->
Name = make_name(Address, Port),
stop_acceptor(whereis(Name)).
-
-stop_system(SysSup) when is_pid(SysSup)->
- exit(SysSup, shutdown).
+
+stop_system(SysSup) ->
+ Name = sshd_sup:system_name(SysSup),
+ spawn(fun() -> sshd_sup:stop_child(Name) end),
+ ok.
stop_system(Address, Port) ->
- stop_system(system_supervisor(Address, Port)).
-
+ spawn(fun() -> sshd_sup:stop_child(Address, Port) end),
+ ok.
+
system_supervisor(Address, Port) ->
Name = make_name(Address, Port),
whereis(Name).
@@ -136,7 +139,7 @@ ssh_acceptor_child_spec(ServerOpts) ->
Port = proplists:get_value(port, ServerOpts),
Name = id(ssh_acceptor_sup, Address, Port),
StartFunc = {ssh_acceptor_sup, start_link, [ServerOpts]},
- Restart = permanent,
+ Restart = transient,
Shutdown = infinity,
Modules = [ssh_acceptor_sup],
Type = supervisor,
diff --git a/lib/ssh/src/sshd_sup.erl b/lib/ssh/src/sshd_sup.erl
index 747906b2cf..60222f5172 100644
--- a/lib/ssh/src/sshd_sup.erl
+++ b/lib/ssh/src/sshd_sup.erl
@@ -1,7 +1,7 @@
%%
%% %CopyrightBegin%
%%
-%% Copyright Ericsson AB 2008-2010. All Rights Reserved.
+%% Copyright Ericsson AB 2008-2013. 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
@@ -58,12 +58,7 @@ start_child(ServerOpts) ->
end.
stop_child(Name) ->
- case supervisor:terminate_child(?MODULE, Name) of
- ok ->
- supervisor:delete_child(?MODULE, Name);
- Error ->
- Error
- end.
+ supervisor:terminate_child(?MODULE, Name).
stop_child(Address, Port) ->
Name = id(Address, Port),
@@ -94,7 +89,7 @@ init([Servers]) ->
child_spec(Address, Port, ServerOpts) ->
Name = id(Address, Port),
StartFunc = {ssh_system_sup, start_link, [ServerOpts]},
- Restart = transient,
+ Restart = temporary,
Shutdown = infinity,
Modules = [ssh_system_sup],
Type = supervisor,
diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl
index b3281e433e..b4e3871efd 100644
--- a/lib/ssh/test/ssh_basic_SUITE.erl
+++ b/lib/ssh/test/ssh_basic_SUITE.erl
@@ -46,7 +46,7 @@ all() ->
daemon_already_started,
server_password_option,
server_userpassword_option,
- close].
+ double_close].
groups() ->
[{dsa_key, [], basic_tests()},
@@ -57,7 +57,7 @@ groups() ->
].
basic_tests() ->
- [send, peername_sockname,
+ [send, close, peername_sockname,
exec, exec_compressed, shell, cli, known_hosts,
idle_time, rekey, openssh_zlib_basic_test].
@@ -487,7 +487,7 @@ internal_error(Config) when is_list(Config) ->
{Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir},
{user_dir, UserDir},
{failfun, fun ssh_test_lib:failfun/2}]),
- {error,Error} =
+ {error, Error} =
ssh:connect(Host, Port, [{silently_accept_hosts, true},
{user_dir, UserDir},
{user_interaction, false}]),
@@ -566,9 +566,35 @@ ips(Name) when is_list(Name) ->
ordsets:from_list(IPs4++IPs6).
%%--------------------------------------------------------------------
+
close() ->
- [{doc, "Simulate that we try to close an already closed connection"}].
+ [{doc, "Client receives close when server closes"}].
close(Config) when is_list(Config) ->
+ process_flag(trap_exit, true),
+ SystemDir = filename:join(?config(priv_dir, Config), system),
+ UserDir = ?config(priv_dir, Config),
+
+ {Server, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir},
+ {user_dir, UserDir},
+ {failfun, fun ssh_test_lib:failfun/2}]),
+ Client =
+ ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
+ {user_dir, UserDir},
+ {user_interaction, false}]),
+ {ok, ChannelId} = ssh_connection:session_channel(Client, infinity),
+
+ ssh:stop_daemon(Server),
+ receive
+ {ssh_cm, Client,{closed, ChannelId}} ->
+ ok
+ after 5000 ->
+ ct:fail(timeout)
+ end.
+
+%%--------------------------------------------------------------------
+double_close() ->
+ [{doc, "Simulate that we try to close an already closed connection"}].
+double_close(Config) when is_list(Config) ->
SystemDir = ?config(data_dir, Config),
PrivDir = ?config(priv_dir, Config),
UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
@@ -587,6 +613,8 @@ close(Config) when is_list(Config) ->
exit(CM, {shutdown, normal}),
ok = ssh:close(CM).
+%%--------------------------------------------------------------------
+
openssh_zlib_basic_test() ->
[{doc, "Test basic connection with openssh_zlib"}].
openssh_zlib_basic_test(Config) ->
@@ -612,7 +640,7 @@ openssh_zlib_basic_test(Config) ->
%% the "tcp-application" before the socket closed message is recived
check_error("Internal error") ->
ok;
-check_error("Connection Lost") ->
+check_error("Connection closed") ->
ok;
check_error(Error) ->
ct:fail(Error).