Allow supervisor:terminate_child(SupRef,Pid) for simple_one_for_one

supervisor:terminate_child/2 was not allowed if the supervisor used
restart strategy simple_one_for_one. This is now changed so that
children of this type of supervisors can be terminated by specifying
the child's Pid.

3 files changed, 94 insertions, 39 deletions

diff --git a/lib/stdlib/doc/src/supervisor.xml b/lib/stdlib/doc/src/supervisor.xml
index 45fa0847a8..d6203bdaa0 100644
--- a/lib/stdlib/doc/src/supervisor.xml
+++ b/lib/stdlib/doc/src/supervisor.xml
@@ -4,7 +4,7 @@
 <erlref>
   <header>
     <copyright>
-      <year>1996</year><year>2010</year>
+      <year>1996</year><year>2011</year>
       <holder>Ericsson AB. All Rights Reserved.</holder>
     </copyright>
     <legalnotice>
@@ -83,11 +83,17 @@
           supervisor, where all child processes are dynamically added
           instances of the same process type, i.e. running the same
           code.</p>
-        <p>The functions <c>terminate_child/2</c>, <c>delete_child/2</c>
+        <p>The functions <c>delete_child/2</c>
           and <c>restart_child/2</c> are invalid for
           <c>simple_one_for_one</c> supervisors and will return
           <c>{error,simple_one_for_one}</c> if the specified supervisor
           uses this restart strategy.</p>
+        <p>The function <c>terminate_child/2</c> can be used for
+          children under <c>simple_one_for_one</c> supervisors by
+          giving the child's <c>pid()</c> as the second argument. If
+          instead the child specification identifier is used,
+          <c>terminate_child/2</c> will return
+          <c>{error,simple_one_for_one}</c>.</p>
       </item>
     </list>
     <p>To prevent a supervisor from getting into an infinite loop of
@@ -311,24 +317,33 @@ child_spec() = {Id,StartFunc,Restart,Shutdown,Type,Modules}
       <type>
         <v>SupRef = Name | {Name,Node} | {global,Name} | pid()</v>
         <v>&nbsp;Name = Node = atom()</v>
-        <v>Id = term()</v>
+        <v>Id = pid() | term()</v>
         <v>Result = ok | {error,Error}</v>
         <v>&nbsp;Error = not_found | simple_one_for_one</v>
       </type>
       <desc>
-        <p>Tells the supervisor <c>SupRef</c> to terminate the child
-          process corresponding to the child specification identified
-          by <c>Id</c>. The process, if there is one, is terminated but
-          the child specification is kept by the supervisor. This means
-          that the child process may be later be restarted by
-          the supervisor. The child process can also be restarted
-          explicitly by calling <c>restart_child/2</c>. Use
-          <c>delete_child/2</c> to remove the child specification.</p>
+        <p>Tells the supervisor <c>SupRef</c> to terminate the given
+          child.</p>
+        <p>If the supervisor is not <c>simple_one_for_one</c>,
+          <c>Id</c> must be the child specification identifier. The
+          process, if there is one, is terminated but the child
+          specification is kept by the supervisor. The child process
+          may later be restarted by the supervisor. The child process
+          can also be restarted explicitly by calling
+          <c>restart_child/2</c>. Use <c>delete_child/2</c> to remove
+          the child specification.</p>
+        <p>If the supervisor is <c>simple_one_for_one</c>, <c>Id</c>
+          must be the child process' <c>pid()</c>. I the specified
+          process is alive, but is not a child of the given
+          supervisor, the function will return
+          <c>{error,not_found}</c>. If the child specification
+          identifier is given instead instead of a <c>pid()</c>, the
+          function will return <c>{error,simple_one_for_one}</c>.</p>
+        <p>If successful, the function returns <c>ok</c>. If there is
+          no child specification with the specified <c>Id</c>, the
+          function returns <c>{error,not_found}</c>.</p>
         <p>See <c>start_child/2</c> for a description of
           <c>SupRef</c>.</p>
-        <p>If successful, the function returns <c>ok</c>. If there is
-          no child specification with the specified <c>Id</c>,
-          the function returns <c>{error,not_found}</c>.</p>
       </desc>
     </func>
     <func>
diff --git a/lib/stdlib/src/supervisor.erl b/lib/stdlib/src/supervisor.erl
index 368dc2e3e5..4fd7f1d47c 100644
--- a/lib/stdlib/src/supervisor.erl
+++ b/lib/stdlib/src/supervisor.erl
@@ -138,7 +138,7 @@ delete_child(Supervisor, Name) ->
 %%-----------------------------------------------------------------
 
 -type term_err() :: 'not_found' | 'simple_one_for_one'.
--spec terminate_child(sup_ref(), term()) -> 'ok' | {'error', term_err()}.
+-spec terminate_child(sup_ref(), pid() | term()) -> 'ok' | {'error', term_err()}.
 terminate_child(Supervisor, Name) ->
     call(Supervisor, {terminate_child, Name}).
 
@@ -297,8 +297,26 @@ handle_call({start_child, EArgs}, _From, State) when ?is_simple(State) ->
 	    {reply, What, State}
     end;
 
-%%% The requests terminate_child, delete_child and restart_child are 
-%%% invalid for simple_one_for_one supervisors.
+%% terminate_child for simple_one_for_one can only be done with pid
+handle_call({terminate_child, Name}, _From, State) when not is_pid(Name),
+							?is_simple(State) ->
+    {reply, {error, simple_one_for_one}, State};
+
+handle_call({terminate_child, Name}, _From, State) ->
+    case get_child(Name, State, ?is_simple(State)) of
+	{value, Child} ->
+	    case do_terminate(Child, State#state.name) of
+		#child{restart_type=RT} when RT=:=temporary; ?is_simple(State) ->
+		    {reply, ok, state_del_child(Child, State)};
+		NChild ->
+		    {reply, ok, replace_child(NChild, State)}
+		end;
+	false ->
+	    {reply, {error, not_found}, State}
+    end;
+
+%%% The requests delete_child and restart_child are invalid for
+%%% simple_one_for_one supervisors.
 handle_call({_Req, _Data}, _From, State) when ?is_simple(State) ->
     {reply, {error, simple_one_for_one}, State};
 
@@ -341,19 +359,6 @@ handle_call({delete_child, Name}, _From, State) ->
 	    {reply, {error, not_found}, State}
     end;
 
-handle_call({terminate_child, Name}, _From, State) ->
-    case get_child(Name, State) of
-	{value, Child} ->
-	    case do_terminate(Child, State#state.name) of
-		#child{restart_type = temporary} = NChild ->
-		    {reply, ok, state_del_child(NChild, State)};
-		NChild ->
-		    {reply, ok, replace_child(NChild, State)}
-		end;
-	_ ->
-	    {reply, {error, not_found}, State}
-    end;
-
 handle_call(which_children, _From, #state{children = [#child{restart_type = temporary,
 							     child_type = CT,
 							     modules = Mods}]} =
@@ -849,7 +854,28 @@ split_child(_, [], After) ->
     {lists:reverse(After), []}.
 
 get_child(Name, State) ->
+    get_child(Name, State, false).
+get_child(Pid, State, AllowPid) when AllowPid, is_pid(Pid) ->
+    get_dynamic_child(Pid, State);
+get_child(Name, State, _) ->
     lists:keysearch(Name, #child.name, State#state.children).
+
+get_dynamic_child(Pid, #state{children=[Child], dynamics=Dynamics}) ->
+    case is_dynamic_pid(Pid, dynamics_db(Child#child.restart_type, Dynamics)) of
+	true ->
+	    {value, Child#child{pid=Pid}};
+	false ->
+	    case erlang:is_process_alive(Pid) of
+		true -> false;
+		false -> {value, Child}
+	    end
+    end.
+
+is_dynamic_pid(Pid, Dynamics) when is_list(Dynamics) ->
+    lists:member(Pid, Dynamics);
+is_dynamic_pid(Pid, Dynamics) ->
+    dict:is_key(Pid, Dynamics).
+
 replace_child(Child, State) ->
     Chs = do_replace_child(Child, State#state.children),
     State#state{children = Chs}.
diff --git a/lib/stdlib/test/supervisor_SUITE.erl b/lib/stdlib/test/supervisor_SUITE.erl
index f9ceed8f84..cc271bd047 100644
--- a/lib/stdlib/test/supervisor_SUITE.erl
+++ b/lib/stdlib/test/supervisor_SUITE.erl
@@ -20,7 +20,7 @@
 
 -module(supervisor_SUITE).
 
--include_lib("test_server/include/test_server.hrl").
+-include_lib("common_test/include/ct.hrl").
 -define(TIMEOUT, 1000).
 
 %% Testserver specific export
@@ -349,8 +349,7 @@ child_adm(Config) when is_list(Config) ->
     ok = supervisor:terminate_child(sup_test, child1),
 
     %% Start of already existing but not running process 
-    {error,already_present} =
-	supervisor:start_child(sup_test, Child),
+    {error,already_present} = supervisor:start_child(sup_test, Child),
 
     %% Restart
     {ok, CPid2} = supervisor:restart_child(sup_test, child1),
@@ -377,6 +376,11 @@ child_adm(Config) when is_list(Config) ->
     [{child1, CPid3, worker, []}] = supervisor:which_children(sup_test),
     [1,1,0,1] = get_child_counts(sup_test),
 
+    %% Terminate with Pid not allowed when not simple_one_for_one
+    {error,not_found} = supervisor:terminate_child(sup_test, CPid3),
+    [{child1, CPid3, worker, []}] = supervisor:which_children(sup_test),
+    [1,1,0,1] = get_child_counts(sup_test),
+
     {'EXIT',{noproc,{gen_server,call,[foo,which_children,infinity]}}}
 	= (catch supervisor:which_children(foo)),
     {'EXIT',{noproc,{gen_server,call,[foo,count_children,infinity]}}}
@@ -412,16 +416,26 @@ child_adm_simple(Config) when is_list(Config) ->
     [1,2,0,2] = get_child_counts(sup_test),
 
     %% Termination
-    {error, simple_one_for_one} =
-	supervisor:terminate_child(sup_test, child1),
+    {error, simple_one_for_one} = supervisor:terminate_child(sup_test, child1),
+    [1,2,0,2] = get_child_counts(sup_test),
+    ok = supervisor:terminate_child(sup_test,CPid1),
+    [_] = supervisor:which_children(sup_test),
+    [1,1,0,1] = get_child_counts(sup_test),
+    false = erlang:is_process_alive(CPid1),
+    %% Terminate non-existing proccess is ok
+    ok = supervisor:terminate_child(sup_test,CPid1),
+    [_] = supervisor:which_children(sup_test),
+    [1,1,0,1] = get_child_counts(sup_test),
+    %% Terminate pid which is not a child of this supervisor is not ok
+    NoChildPid = spawn_link(fun() -> receive after infinity -> ok end end),
+    {error, not_found} = supervisor:terminate_child(sup_test, NoChildPid),
+    true = erlang:is_process_alive(NoChildPid),
 
     %% Restart
-    {error, simple_one_for_one} =
-	supervisor:restart_child(sup_test, child1),
+    {error, simple_one_for_one} = supervisor:restart_child(sup_test, child1),
 
     %% Deletion
-    {error, simple_one_for_one} =
-	supervisor:delete_child(sup_test, child1),
+    {error, simple_one_for_one} = supervisor:delete_child(sup_test, child1),
     ok.
 
 %%-------------------------------------------------------------------------