Don't save child spec for temporary child if child's start func returns ignore

Supervisor should never keep child specs for dead temporary children.

3 files changed, 72 insertions, 18 deletions

diff --git a/lib/stdlib/doc/src/supervisor.xml b/lib/stdlib/doc/src/supervisor.xml
index cddb55e5c5..33a7f5bb6a 100644
--- a/lib/stdlib/doc/src/supervisor.xml
+++ b/lib/stdlib/doc/src/supervisor.xml
@@ -127,25 +127,18 @@ child_spec() = {Id,StartFunc,Restart,Shutdown,Type,Modules}
         <p><c>StartFunc</c> defines the function call used to start
           the child process. It should be a module-function-arguments
           tuple <c>{M,F,A}</c> used as <c>apply(M,F,A)</c>.</p>
-        <p>          <br></br>
-</p>
         <p>The start function <em>must create and link to</em> the child
           process, and should return <c>{ok,Child}</c> or
           <c>{ok,Child,Info}</c> where <c>Child</c> is the pid of
           the child process and <c>Info</c> an arbitrary term which is
           ignored by the supervisor.</p>
-        <p>          <br></br>
-</p>
         <p>The start function can also return <c>ignore</c> if the child
           process for some reason cannot be started, in which case
-          the child specification will be kept by the supervisor but
-          the non-existing child process will be ignored.</p>
-        <p>          <br></br>
-</p>
+          the child specification will be kept by the supervisor
+	  (unless it is a temporary child) but the non-existing child
+	  process will be ignored.</p>
         <p>If something goes wrong, the function may also return an
           error tuple <c>{error,Error}</c>.</p>
-        <p>          <br></br>
-</p>
         <p>Note that the <c>start_link</c> functions of the different
           behaviour modules fulfill the above requirements.</p>
       </item>
diff --git a/lib/stdlib/src/supervisor.erl b/lib/stdlib/src/supervisor.erl
index 42ea42f42e..ac5b078c29 100644
--- a/lib/stdlib/src/supervisor.erl
+++ b/lib/stdlib/src/supervisor.erl
@@ -270,6 +270,8 @@ start_children(Children, SupName) -> start_children(Children, [], SupName).
 
 start_children([Child|Chs], NChildren, SupName) ->
     case do_start_child(SupName, Child) of
+	{ok, undefined} when Child#child.restart_type =:= temporary ->
+	    start_children(Chs, NChildren, SupName);
 	{ok, Pid} ->
 	    start_children(Chs, [Child#child{pid = Pid}|NChildren], SupName);
 	{ok, Pid, _Extra} ->
@@ -325,6 +327,8 @@ handle_call({start_child, EArgs}, _From, State) when ?is_simple(State) ->
     #child{mfargs = {M, F, A}} = Child,
     Args = A ++ EArgs,
     case do_start_child_i(M, F, Args) of
+	{ok, undefined} when Child#child.restart_type =:= temporary ->
+	    {reply, {ok, undefined}, State};
 	{ok, Pid} ->
 	    NState = save_dynamic_child(Child#child.restart_type, Pid, Args, State),
 	    {reply, {ok, Pid}, NState};
@@ -611,12 +615,12 @@ handle_start_child(Child, State) ->
     case get_child(Child#child.name, State) of
 	false ->
 	    case do_start_child(State#state.name, Child) of
+		{ok, undefined} when Child#child.restart_type =:= temporary ->
+		    {{ok, undefined}, State};
 		{ok, Pid} ->
-		    {{ok, Pid},
-		     save_child(Child#child{pid = Pid}, State)};
+		    {{ok, Pid}, save_child(Child#child{pid = Pid}, State)};
 		{ok, Pid, Extra} ->
-		    {{ok, Pid, Extra},
-		     save_child(Child#child{pid = Pid}, State)};
+		    {{ok, Pid, Extra}, save_child(Child#child{pid = Pid}, State)};
 		{error, What} ->
 		    {{error, {What, Child}}, State}
 	    end;
diff --git a/lib/stdlib/test/supervisor_SUITE.erl b/lib/stdlib/test/supervisor_SUITE.erl
index fa6faa66f2..71b76c093f 100644
--- a/lib/stdlib/test/supervisor_SUITE.erl
+++ b/lib/stdlib/test/supervisor_SUITE.erl
@@ -34,8 +34,10 @@
 
 %% API tests
 -export([ sup_start_normal/1, sup_start_ignore_init/1, 
-	  sup_start_ignore_child/1, sup_start_error_return/1,
-	  sup_start_fail/1, sup_stop_infinity/1,
+	  sup_start_ignore_child/1, sup_start_ignore_temporary_child/1,
+	  sup_start_ignore_temporary_child_start_child/1,
+	  sup_start_ignore_temporary_child_start_child_simple/1,
+	  sup_start_error_return/1, sup_start_fail/1, sup_stop_infinity/1,
 	  sup_stop_timeout/1, sup_stop_brutal_kill/1, child_adm/1,
 	  child_adm_simple/1, child_specs/1, extra_return/1]).
 
@@ -85,8 +87,10 @@ all() ->
 groups() -> 
     [{sup_start, [],
       [sup_start_normal, sup_start_ignore_init,
-       sup_start_ignore_child, sup_start_error_return,
-       sup_start_fail]},
+       sup_start_ignore_child, sup_start_ignore_temporary_child,
+       sup_start_ignore_temporary_child_start_child,
+       sup_start_ignore_temporary_child_start_child_simple,
+       sup_start_error_return, sup_start_fail]},
      {sup_stop, [],
       [sup_stop_infinity, sup_stop_timeout,
        sup_stop_brutal_kill]},
@@ -191,6 +195,59 @@ sup_start_ignore_child(Config) when is_list(Config) ->
     [2,1,0,2] = get_child_counts(sup_test).
 
 %%-------------------------------------------------------------------------
+%% Tests what happens if child's init-callback returns ignore for a
+%% temporary child when ChildSpec is returned directly from supervisor
+%% init callback.
+%% Child spec shall NOT be saved!!!
+sup_start_ignore_temporary_child(Config) when is_list(Config) ->
+    process_flag(trap_exit, true),
+    Child1 = {child1, {supervisor_1, start_child, [ignore]},
+	      temporary, 1000, worker, []},
+    Child2 = {child2, {supervisor_1, start_child, []}, temporary,
+	      1000, worker, []},
+    {ok, _Pid}  = start_link({ok, {{one_for_one, 2, 3600}, [Child1,Child2]}}),
+
+    [{child2, CPid2, worker, []}] = supervisor:which_children(sup_test),
+    true = is_pid(CPid2),
+    [1,1,0,1] = get_child_counts(sup_test).
+
+%%-------------------------------------------------------------------------
+%% Tests what happens if child's init-callback returns ignore for a
+%% temporary child when child is started with start_child/2.
+%% Child spec shall NOT be saved!!!
+sup_start_ignore_temporary_child_start_child(Config) when is_list(Config) ->
+    process_flag(trap_exit, true),
+    {ok, _Pid}  = start_link({ok, {{one_for_one, 2, 3600}, []}}),
+    Child1 = {child1, {supervisor_1, start_child, [ignore]},
+	      temporary, 1000, worker, []},
+    Child2 = {child2, {supervisor_1, start_child, []}, temporary,
+	      1000, worker, []},
+
+    {ok, undefined} = supervisor:start_child(sup_test, Child1),
+    {ok, CPid2} = supervisor:start_child(sup_test, Child2),
+
+    [{child2, CPid2, worker, []}] = supervisor:which_children(sup_test),
+    [1,1,0,1] = get_child_counts(sup_test).
+
+%%-------------------------------------------------------------------------
+%% Tests what happens if child's init-callback returns ignore for a
+%% temporary child when child is started with start_child/2, and the
+%% supervisor is simple_one_for_one.
+%% Child spec shall NOT be saved!!!
+sup_start_ignore_temporary_child_start_child_simple(Config)
+  when is_list(Config) ->
+    process_flag(trap_exit, true),
+    Child1 = {child1, {supervisor_1, start_child, [ignore]},
+	      temporary, 1000, worker, []},
+    {ok, _Pid}  = start_link({ok, {{simple_one_for_one, 2, 3600}, [Child1]}}),
+
+    {ok, undefined} = supervisor:start_child(sup_test, []),
+    {ok, CPid2} = supervisor:start_child(sup_test, []),
+
+    [{undefined, CPid2, worker, []}] = supervisor:which_children(sup_test),
+    [1,1,0,1] = get_child_counts(sup_test).
+
+%%-------------------------------------------------------------------------
 %% Tests what happens if init-callback returns a invalid value.
 sup_start_error_return(Config) when is_list(Config) ->
     process_flag(trap_exit, true),