From 6c61c2169e635bcf100e128096f66a9334035c7b Mon Sep 17 00:00:00 2001 From: Stefan Grundmann Date: Wed, 1 Sep 2010 19:42:57 +0200 Subject: fix process leak in ssh_system_sup (dynamicaly created childs where not cleaned up) The ssh_system_sup supervisor supervises one ssh_subsystem_sup process for every client connection. There was no functionality to free resources (terminate_child/ delete_child) when a client connection was closed. Which lead to one ssh_subsystem_sup and one ssh_channel_sup process left over. This commit adds ssh_system_sup:stop_subsystem/2 and code that calls it in ssh_connection_manager:terminate/2. --- lib/ssh/src/ssh_connection_manager.erl | 7 ++++++- lib/ssh/src/ssh_system_sup.erl | 13 ++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh_connection_manager.erl b/lib/ssh/src/ssh_connection_manager.erl index 9e55312e5f..6bf89224cf 100644 --- a/lib/ssh/src/ssh_connection_manager.erl +++ b/lib/ssh/src/ssh_connection_manager.erl @@ -560,13 +560,18 @@ handle_info({'EXIT', _, _}, State) -> %% The return value is ignored. %%-------------------------------------------------------------------- terminate(Reason, #state{connection_state = - #connection{requests = Requests}, + #connection{requests = Requests, + 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. %%-------------------------------------------------------------------- diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl index 477f60f993..9336f59444 100644 --- a/lib/ssh/src/ssh_system_sup.erl +++ b/lib/ssh/src/ssh_system_sup.erl @@ -33,7 +33,7 @@ stop_system/2, system_supervisor/2, subsystem_supervisor/1, channel_supervisor/1, connection_supervisor/1, - acceptor_supervisor/1, start_subsystem/2, restart_subsystem/2, restart_acceptor/2]). + acceptor_supervisor/1, start_subsystem/2, restart_subsystem/2, restart_acceptor/2, stop_subsystem/2]). %% Supervisor callback -export([init/1]). @@ -83,6 +83,17 @@ start_subsystem(SystemSup, Options) -> Spec = ssh_subsystem_child_spec(Options), supervisor:start_child(SystemSup, Spec). +stop_subsystem(SystemSup, SubSys) -> + case lists:keyfind(SubSys, 2, supervisor:which_children(SystemSup)) of + false -> + {error, not_found}; + {Id, _, _, _} -> + spawn(fun() -> supervisor:terminate_child(SystemSup, Id), + supervisor:delete_child(SystemSup, Id) end), + ok + end. + + restart_subsystem(Address, Port) -> SysSupName = make_name(Address, Port), SubSysName = id(ssh_subsystem_sup, Address, Port), -- cgit v1.2.3 From fddc1ed0341d13df8373509fa063d889fab8d219 Mon Sep 17 00:00:00 2001 From: nick Date: Thu, 2 Sep 2010 16:47:06 +0200 Subject: Fix race condition when terminating a connection. --- lib/ssh/src/ssh_cli.erl | 28 ++++++++++++++++++++++------ lib/ssh/src/ssh_connection_controler.erl | 4 ++-- lib/ssh/src/ssh_system_sup.erl | 9 ++++++++- 3 files changed, 32 insertions(+), 9 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh_cli.erl b/lib/ssh/src/ssh_cli.erl index 2764ea2e43..57ba87bd42 100644 --- a/lib/ssh/src/ssh_cli.erl +++ b/lib/ssh/src/ssh_cli.erl @@ -419,14 +419,12 @@ start_shell(ConnectionManager, State) -> Shell = State#state.shell, ShellFun = case is_function(Shell) of true -> + {ok, User} = + ssh_userreg:lookup_user(ConnectionManager), case erlang:fun_info(Shell, arity) of {arity, 1} -> - {ok, User} = - ssh_userreg:lookup_user(ConnectionManager), fun() -> Shell(User) end; {arity, 2} -> - {ok, User} = - ssh_userreg:lookup_user(ConnectionManager), {ok, PeerAddr} = ssh_connection_manager:peer_addr(ConnectionManager), fun() -> Shell(User, PeerAddr) end; @@ -441,10 +439,28 @@ start_shell(ConnectionManager, State) -> State#state{group = Group, buf = empty_buf()}. start_shell(_ConnectionManager, Cmd, #state{exec={M, F, A}} = State) -> - Group = group:start(self(), {M, F, A++[Cmd]}, [{echo,false}]), + Group = group:start(self(), {M, F, A++[Cmd]}, [{echo, false}]), + State#state{group = Group, buf = empty_buf()}; +start_shell(ConnectionManager, Cmd, #state{exec=Shell} = State) when is_function(Shell) -> + {ok, User} = + ssh_userreg:lookup_user(ConnectionManager), + ShellFun = + case erlang:fun_info(Shell, arity) of + {arity, 1} -> + fun() -> Shell(Cmd) end; + {arity, 2} -> + fun() -> Shell(Cmd, User) end; + {arity, 3} -> + {ok, PeerAddr} = + ssh_connection_manager:peer_addr(ConnectionManager), + fun() -> Shell(Cmd, User, PeerAddr) end; + _ -> + Shell + end, + Echo = get_echo(State#state.pty), + Group = group:start(self(), ShellFun, [{echo,Echo}]), State#state{group = Group, buf = empty_buf()}. - % Pty can be undefined if the client never sets any pty options before % starting the shell. get_echo(undefined) -> diff --git a/lib/ssh/src/ssh_connection_controler.erl b/lib/ssh/src/ssh_connection_controler.erl index 636ecba532..ca3e62dc83 100644 --- a/lib/ssh/src/ssh_connection_controler.erl +++ b/lib/ssh/src/ssh_connection_controler.erl @@ -126,8 +126,8 @@ handle_cast(_, State) -> %% handle_info(ssh_connected, State) -> %% {stop, normal, State}; %% Servant termination. -handle_info({'EXIT', _Pid, normal}, State) -> - {stop, normal, State}. +handle_info({'EXIT', _Pid, Reason}, State) -> + {stop, Reason, State}. %%----------------------------------------------------------------- %% Func: code_change/3 diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl index 9336f59444..0ff73f1648 100644 --- a/lib/ssh/src/ssh_system_sup.erl +++ b/lib/ssh/src/ssh_system_sup.erl @@ -33,7 +33,8 @@ stop_system/2, system_supervisor/2, subsystem_supervisor/1, channel_supervisor/1, connection_supervisor/1, - acceptor_supervisor/1, start_subsystem/2, restart_subsystem/2, restart_acceptor/2, stop_subsystem/2]). + acceptor_supervisor/1, start_subsystem/2, restart_subsystem/2, + restart_acceptor/2, stop_subsystem/2]). %% Supervisor callback -export([init/1]). @@ -90,6 +91,12 @@ stop_subsystem(SystemSup, SubSys) -> {Id, _, _, _} -> spawn(fun() -> supervisor:terminate_child(SystemSup, Id), supervisor:delete_child(SystemSup, Id) end), + ok; + {'EXIT', {noproc, _}} -> + %% Already terminated; probably shutting down. + ok; + {'EXIT', {shutdown, _}} -> + %% Already shutting down. ok end. -- cgit v1.2.3 From 24c67e110e2219c269454566126ad0885ee57bdc Mon Sep 17 00:00:00 2001 From: Niclas Eklund Date: Wed, 6 Oct 2010 15:19:22 +0200 Subject: In some cases a crash report was generated when a connection was closing down. This was caused by a race condition between two processes. --- lib/ssh/src/ssh.appup.src | 12 ++++++------ lib/ssh/src/ssh_connection_handler.erl | 16 ++++++++++++---- lib/ssh/vsn.mk | 7 +++++-- 3 files changed, 23 insertions(+), 12 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/src/ssh.appup.src b/lib/ssh/src/ssh.appup.src index 82114c9afd..09249e5e39 100644 --- a/lib/ssh/src/ssh.appup.src +++ b/lib/ssh/src/ssh.appup.src @@ -19,9 +19,9 @@ {"%VSN%", [ - {"1.1.10", [{load_module, ssh_connection_manager, soft_purge, soft_purge, []}]}, - {"1.1.9", [{load_module, ssh_channel, soft_purge, soft_purge, []}, - {load_module, ssh_connection_manager, soft_purge, soft_purge, []}]}, + {"1.1.11", [{restart_application, ssh}]}, + {"1.1.10", [{restart_application, ssh}]}, + {"1.1.9", [{restart_application, ssh}]}, {"1.1.8", [{restart_application, ssh}]}, {"1.1.7", [{restart_application, ssh}]}, {"1.1.6", [{restart_application, ssh}]}, @@ -31,9 +31,9 @@ {"1.1.2", [{restart_application, ssh}]} ], [ - {"1.1.10", [{load_module, ssh_connection_manager, soft_purge, soft_purge, []}]}, - {"1.1.9", [{load_module, ssh_channel, soft_purge, soft_purge, []}, - {load_module, ssh_connection_manager, soft_purge, soft_purge, []}]}, + {"1.1.11", [{restart_application, ssh}]}, + {"1.1.10", [{restart_application, ssh}]}, + {"1.1.9", [{restart_application, ssh}]}, {"1.1.8", [{restart_application, ssh}]}, {"1.1.7", [{restart_application, ssh}]}, {"1.1.6", [{restart_application, ssh}]}, diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 822ef8f8f9..926d4fddce 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -705,11 +705,19 @@ generate_event(<> = Msg, StateName, Byte == ?SSH_MSG_CHANNEL_REQUEST; Byte == ?SSH_MSG_CHANNEL_SUCCESS; Byte == ?SSH_MSG_CHANNEL_FAILURE -> - ssh_connection_manager:event(Pid, Msg), - State = generate_event_new_state(State0, EncData), - next_packet(State), - {next_state, StateName, State}; + try + ssh_connection_manager:event(Pid, Msg), + State = generate_event_new_state(State0, EncData), + 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} + end; generate_event(Msg, StateName, State0, EncData) -> Event = ssh_bits:decode(Msg), State = generate_event_new_state(State0, EncData), diff --git a/lib/ssh/vsn.mk b/lib/ssh/vsn.mk index 8e31851a8e..cf90e3b11e 100644 --- a/lib/ssh/vsn.mk +++ b/lib/ssh/vsn.mk @@ -1,9 +1,12 @@ #-*-makefile-*- ; force emacs to enter makefile-mode -SSH_VSN = 1.1.11 +SSH_VSN = 1.1.12 APP_VSN = "ssh-$(SSH_VSN)" -TICKETS = OTP-8735 +TICKETS = OTP-8807 \ + OTP-8881 + +TICKETS_1.1.11 = OTP-8735 TICKETS_1.1.10 = OTP-8714 -- cgit v1.2.3 From 644d0c54643eda5216a1fa0ff012d1ebcae5ce8e Mon Sep 17 00:00:00 2001 From: Erlang/OTP Date: Thu, 21 Oct 2010 14:11:20 +0200 Subject: Update release notes --- lib/ssh/doc/src/notes.xml | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) (limited to 'lib/ssh') diff --git a/lib/ssh/doc/src/notes.xml b/lib/ssh/doc/src/notes.xml index ce18cabfb5..aedb682f79 100644 --- a/lib/ssh/doc/src/notes.xml +++ b/lib/ssh/doc/src/notes.xml @@ -29,7 +29,38 @@ notes.xml -
Ssh 1.1.11 +
Ssh 1.1.12 + +
Fixed Bugs and Malfunctions + + +

+ The processes ssh_subsystem_sup and one ssh_channel_sup + was not terminated when a connection was closed.

+

+ Own Id: OTP-8807

+ + +

+ The ssh_system_sup did not catch noproc and shutdown + messages.

+

+ Own Id: OTP-8863

+ + +

+ In some cases a crash report was generated when a + connection was closing down. This was caused by a race + condition between two processes.

+

+ Own Id: OTP-8881 Aux Id: seq11656, seq11648

+ + +
+ +
+ +
Ssh 1.1.11
Fixed Bugs and Malfunctions -- cgit v1.2.3 From 00d27caa001444e65e9dd3a7d3bc65cfde4b5866 Mon Sep 17 00:00:00 2001 From: Niclas Eklund Date: Mon, 25 Oct 2010 12:16:36 +0200 Subject: The fix regarding OTP-8863 was not included in the previous version as stated --- lib/ssh/doc/src/notes.xml | 16 ++++++++++++++++ lib/ssh/src/ssh.appup.src | 2 ++ lib/ssh/src/ssh_system_sup.erl | 4 ++-- lib/ssh/vsn.mk | 6 ++++-- 4 files changed, 24 insertions(+), 4 deletions(-) (limited to 'lib/ssh') diff --git a/lib/ssh/doc/src/notes.xml b/lib/ssh/doc/src/notes.xml index aedb682f79..9a08c72c93 100644 --- a/lib/ssh/doc/src/notes.xml +++ b/lib/ssh/doc/src/notes.xml @@ -29,6 +29,22 @@ notes.xml +
Ssh 1.1.13 + +
Fixed Bugs and Malfunctions + + +

+ The fix regarding OTP-8863 was not included in the previous + version as stated.

+

+ Own Id: OTP-8908

+ + +
+ +
+
Ssh 1.1.12
Fixed Bugs and Malfunctions diff --git a/lib/ssh/src/ssh.appup.src b/lib/ssh/src/ssh.appup.src index 09249e5e39..160e336873 100644 --- a/lib/ssh/src/ssh.appup.src +++ b/lib/ssh/src/ssh.appup.src @@ -19,6 +19,7 @@ {"%VSN%", [ + {"1.1.12", [{load_module, ssh_system_sup, soft_purge, soft_purge, []}]}, {"1.1.11", [{restart_application, ssh}]}, {"1.1.10", [{restart_application, ssh}]}, {"1.1.9", [{restart_application, ssh}]}, @@ -31,6 +32,7 @@ {"1.1.2", [{restart_application, ssh}]} ], [ + {"1.1.12", [{load_module, ssh_system_sup, soft_purge, soft_purge, []}]}, {"1.1.11", [{restart_application, ssh}]}, {"1.1.10", [{restart_application, ssh}]}, {"1.1.9", [{restart_application, ssh}]}, diff --git a/lib/ssh/src/ssh_system_sup.erl b/lib/ssh/src/ssh_system_sup.erl index 0ff73f1648..d1003e12f2 100644 --- a/lib/ssh/src/ssh_system_sup.erl +++ b/lib/ssh/src/ssh_system_sup.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2008-2009. All Rights Reserved. +%% Copyright Ericsson AB 2008-2010. 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 @@ -85,7 +85,7 @@ start_subsystem(SystemSup, Options) -> supervisor:start_child(SystemSup, Spec). stop_subsystem(SystemSup, SubSys) -> - case lists:keyfind(SubSys, 2, supervisor:which_children(SystemSup)) of + case catch lists:keyfind(SubSys, 2, supervisor:which_children(SystemSup)) of false -> {error, not_found}; {Id, _, _, _} -> diff --git a/lib/ssh/vsn.mk b/lib/ssh/vsn.mk index cf90e3b11e..a6bc521003 100644 --- a/lib/ssh/vsn.mk +++ b/lib/ssh/vsn.mk @@ -1,9 +1,11 @@ #-*-makefile-*- ; force emacs to enter makefile-mode -SSH_VSN = 1.1.12 +SSH_VSN = 1.1.13 APP_VSN = "ssh-$(SSH_VSN)" -TICKETS = OTP-8807 \ +TICKETS = OTP-8908 + +TICKETS_1.1.12 = OTP-8807 \ OTP-8881 TICKETS_1.1.11 = OTP-8735 -- cgit v1.2.3 From 489be05ffa13d66726b3c5d61227294aa1dad992 Mon Sep 17 00:00:00 2001 From: Erlang/OTP Date: Mon, 25 Oct 2010 12:20:40 +0200 Subject: Update version numbers --- lib/ssh/vsn.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/ssh') diff --git a/lib/ssh/vsn.mk b/lib/ssh/vsn.mk index a6bc521003..a053318120 100644 --- a/lib/ssh/vsn.mk +++ b/lib/ssh/vsn.mk @@ -1,6 +1,6 @@ #-*-makefile-*- ; force emacs to enter makefile-mode -SSH_VSN = 1.1.13 +SSH_VSN = 1.1.12 APP_VSN = "ssh-$(SSH_VSN)" TICKETS = OTP-8908 -- cgit v1.2.3