Don't store child that returns ignore in simple supervisor

If a child of a simple_one_for_one returns ignore from its start
function no longer store the child for any restart type. It is not
possible to restart or delete the child because the supervisor is a
simple_one_for_one.

Previously a simple_one_for_one would crash, potentially without
shutting down all of its children, when it tried to shutdown a child
with undefined pid.

Previous only one undefined pid child was stored in a simple_one_for_one
supervisor no matter how many times the child start function returned
ignore.

3 files changed, 33 insertions, 4 deletions

diff --git a/lib/stdlib/doc/src/supervisor.xml b/lib/stdlib/doc/src/supervisor.xml
index ffac1c0bd7..6ff477a42d 100644
--- a/lib/stdlib/doc/src/supervisor.xml
+++ b/lib/stdlib/doc/src/supervisor.xml
@@ -386,9 +386,15 @@
           added to the supervisor and the function returns the same
           value.</p>
         <p>If the child process start function returns <c>ignore</c>,
-          the child specification is added to the supervisor, the pid
-          is set to <c>undefined</c>, and the function returns
-          <c>{ok,undefined}</c>.</p>
+          the child specification is added to the supervisor (unless the
+          supervisor is a <c>simple_one_for_one</c> supervisor, see below),
+          the pid is set to <c>undefined</c> and the function returns
+          <c>{ok,undefined}</c>.
+        </p>
+        <p>In the case of a <c>simple_one_for_one</c> supervisor, when a child
+          process start function returns <c>ignore</c> the functions returns
+          <c>{ok,undefined}</c> and no child is added to the supervisor.
+        </p>
         <p>If the child process start function returns an error tuple or
           an erroneous value, or if it fails, the child specification is
           discarded, and the function returns <c>{error,Error}</c> where
diff --git a/lib/stdlib/src/supervisor.erl b/lib/stdlib/src/supervisor.erl
index 67655b1145..1d7396adee 100644
--- a/lib/stdlib/src/supervisor.erl
+++ b/lib/stdlib/src/supervisor.erl
@@ -381,7 +381,7 @@ 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 ->
+	{ok, undefined} ->
 	    {reply, {ok, undefined}, State};
 	{ok, Pid} ->
 	    NState = save_dynamic_child(Child#child.restart_type, Pid, Args, State),
diff --git a/lib/stdlib/test/supervisor_SUITE.erl b/lib/stdlib/test/supervisor_SUITE.erl
index 9dcf19707c..015b09f35e 100644
--- a/lib/stdlib/test/supervisor_SUITE.erl
+++ b/lib/stdlib/test/supervisor_SUITE.erl
@@ -37,6 +37,7 @@
 	  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_ignore_permanent_child_start_child_simple/1,
 	  sup_start_error_return/1, sup_start_fail/1,
 	  sup_start_map/1, sup_start_map_faulty_specs/1,
 	  sup_stop_infinity/1, sup_stop_timeout/1, sup_stop_brutal_kill/1,
@@ -99,6 +100,7 @@ groups() ->
        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_ignore_permanent_child_start_child_simple,
        sup_start_error_return, sup_start_fail]},
      {sup_start_map, [],
       [sup_start_map, sup_start_map_faulty_specs]},
@@ -250,6 +252,27 @@ sup_start_ignore_temporary_child_start_child_simple(Config)
     [1,1,0,1] = get_child_counts(sup_test).
 
 %%-------------------------------------------------------------------------
+%% Tests what happens if child's init-callback returns ignore for a
+%% permanent 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_permanent_child_start_child_simple(Config)
+  when is_list(Config) ->
+    process_flag(trap_exit, true),
+    Child1 = {child1, {supervisor_1, start_child, [ignore]},
+	      permanent, 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),
+
+    %% Regression test: check that the supervisor terminates without error.
+    exit(Pid, shutdown),
+    check_exit_reason(Pid, shutdown).
+%%-------------------------------------------------------------------------
 %% Tests what happens if init-callback returns a invalid value.
 sup_start_error_return(Config) when is_list(Config) ->
     process_flag(trap_exit, true),