aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authornick <[email protected]>2010-09-03 12:52:02 +0200
committernick <[email protected]>2010-09-03 12:52:02 +0200
commit8f67f51105560f1c0e721117100cf20d0c6061c0 (patch)
treed1c69b96672df5bf985914d9e3799168c41fdf82
parentf0c4e27ef0a5a365cab7b1dc69dbecf7856c3f3f (diff)
parentfddc1ed0341d13df8373509fa063d889fab8d219 (diff)
downloadotp-8f67f51105560f1c0e721117100cf20d0c6061c0.tar.gz
otp-8f67f51105560f1c0e721117100cf20d0c6061c0.tar.bz2
otp-8f67f51105560f1c0e721117100cf20d0c6061c0.zip
Merge branch 'nick/ssh/fix-process-leak/OTP-8807' into dev
* nick/ssh/fix-process-leak/OTP-8807: Fix race condition when terminating a connection. fix process leak in ssh_system_sup (dynamicaly created childs where not cleaned up)
-rw-r--r--lib/ssh/src/ssh_cli.erl28
-rw-r--r--lib/ssh/src/ssh_connection_controler.erl4
-rw-r--r--lib/ssh/src/ssh_connection_manager.erl7
-rw-r--r--lib/ssh/src/ssh_system_sup.erl20
4 files changed, 49 insertions, 10 deletions
diff --git a/lib/ssh/src/ssh_cli.erl b/lib/ssh/src/ssh_cli.erl
index e3b6ffa125..cb78acb84c 100644
--- a/lib/ssh/src/ssh_cli.erl
+++ b/lib/ssh/src/ssh_cli.erl
@@ -415,14 +415,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;
@@ -437,10 +435,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_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 40db2e4adf..f4570b8a48 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]).
+ acceptor_supervisor/1, start_subsystem/2, restart_subsystem/2,
+ restart_acceptor/2, stop_subsystem/2]).
%% Supervisor callback
-export([init/1]).
@@ -83,6 +84,23 @@ 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;
+ {'EXIT', {noproc, _}} ->
+ %% Already terminated; probably shutting down.
+ ok;
+ {'EXIT', {shutdown, _}} ->
+ %% Already shutting down.
+ ok
+ end.
+
+
restart_subsystem(Address, Port) ->
SysSupName = make_name(Address, Port),
SubSysName = id(ssh_subsystem_sup, Address, Port),