Add support for supervisor one_for_all strategy

Adds support for handling restarts for the `one_for_all` strategy that was documented, but lacked
implementation. Makes necessary changes to ensure children are always restarted in the same order
they were originally started, and shutdown in reverse order with last child first, conforming to
OTP behavior.

Closes #1855

Signed-off-by: Winford <[email protected]>

Author: UncleGrumpy

CHANGELOG.md

@@ -92,6 +92,7 @@ instead `badarg`.
 - Fixed a bug where empty atom could not be created on some platforms, thus breaking receiving a message for a registered process from an OTP node.
 - Fix a memory leak in distribution when a BEAM node would monitor a process by name.
 - Fix `list_to_integer`, it was likely buggy with integers close to INT64_MAX
+- Added missing support for supervisor `one_for_all` strategy.
 
 ## [0.6.7] - Unreleased
 

libs/estdlib/src/supervisor.erl

@@ -83,18 +83,20 @@
     }.
 
 -record(child, {
-    pid = undefined,
+    pid = undefined :: pid() | undefined | {restarting, pid()} | {restarting, undefined},
     id :: any(),
     start :: {module(), atom(), [any()] | undefined},
     restart :: restart(),
     shutdown :: shutdown(),
     type :: child_type(),
     modules = [] :: [module()] | dynamic
 }).
--record(state, {restart_strategy :: strategy(), children = [] :: [#child{}]}).
+%% note: the list of children should always be kept in order, with first to start at the head.
+-record(state, {restart_strategy = one_for_one :: strategy(), children = [] :: [#child{}]}).
 
 start_link(Module, Args) ->
     gen_server:start_link(?MODULE, {Module, Args}, []).
+
 start_link(SupName, Module, Args) ->
     gen_server:start_link(SupName, ?MODULE, {Module, Args}, []).
 
@@ -172,7 +174,7 @@ child_spec_to_record(#{id := ChildId, start := MFA} = ChildMap) ->
 init_state([ChildSpec | T], State) ->
     Child = child_spec_to_record(ChildSpec),
     NewChildren = [Child | State#state.children],
-    init_state(T, #state{children = NewChildren});
+    init_state(T, State#state{children = NewChildren});
 init_state([], State) ->
     State#state{children = lists:reverse(State#state.children)}.
 
@@ -182,21 +184,26 @@ start_children([Child | T], StartedC) ->
             start_children(T, [Child#child{pid = Pid} | StartedC])
     end;
 start_children([], StartedC) ->
-    StartedC.
+    %% We should always keep the start list in order for later one_for_all restarts.
+    lists:reverse(StartedC).
 
 handle_call({start_child, ChildSpec}, _From, #state{children = Children} = State) ->
     Child = child_spec_to_record(ChildSpec),
     #child{id = ID} = Child,
     case lists:keyfind(ID, #child.id, State#state.children) of
         #child{pid = undefined} ->
             {reply, {error, already_present}, State};
+        #child{pid = {restarting, _Pid}} ->
+            {reply, {error, restarting}, State};
         #child{pid = Pid} ->
             {reply, {error, {already_started, Pid}}, State};
         false ->
             case try_start(Child) of
                 {ok, Pid, Result} ->
                     UpdatedChild = Child#child{pid = Pid},
-                    {reply, Result, State#state{children = [UpdatedChild | Children]}};
+                    %% The last child to start should always be at the end of the child
+                    %% start list.
+                    {reply, Result, State#state{children = Children ++ [UpdatedChild]}};
                 {error, _Reason} = ErrorT ->
                     {reply, ErrorT, State}
             end
@@ -226,6 +233,8 @@ handle_call({restart_child, ID}, _From, #state{children = Children} = State) ->
                 {error, _Reason} = ErrorT ->
                     {reply, ErrorT, State}
             end;
+        #child{pid = {restarting, _}} = Child ->
+            {reply, {error, restarting}, State};
         #child{} ->
             {reply, {error, running}, State};
         false ->
@@ -236,6 +245,8 @@ handle_call({delete_child, ID}, _From, #state{children = Children} = State) ->
         #child{pid = undefined} ->
             NewChildren = lists:keydelete(ID, #child.id, Children),
             {reply, ok, State#state{children = NewChildren}};
+        #child{pid = {restarting, _}} = Child ->
+            {reply, {error, restarting}, State};
         #child{} ->
             {reply, {error, running}, State};
         false ->
@@ -254,12 +265,7 @@ handle_cast(_Msg, State) ->
     {noreply, State}.
 
 handle_info({'EXIT', Pid, Reason}, State) ->
-    case handle_child_exit(Pid, Reason, State) of
-        {ok, State1} ->
-            {noreply, State1};
-        {shutdown, State1} ->
-            {stop, shutdown, State1}
-    end;
+    handle_child_exit(Pid, Reason, State);
 handle_info({ensure_killed, Pid}, State) ->
     case lists:keyfind(Pid, #child.pid, State#state.children) of
         false ->
@@ -268,13 +274,28 @@ handle_info({ensure_killed, Pid}, State) ->
             exit(Pid, kill),
             {noreply, State}
     end;
+handle_info({restart_many_children, []}, State) ->
+    {noreply, State};
+handle_info({restart_many_children, [#child{pid = Pid} = Child | Children]}, State) ->
+    case try_start(Child) of
+        {ok, NewPid, _Result} ->
+            NewChild = Child#child{pid = NewPid},
+            NewChildren = lists:keyreplace(
+                Pid, #child.pid, State#state.children, NewChild
+            ),
+            {noreply, State#state{children = NewChildren},
+                {timeout, 0, {restart_many_children, Children}}};
+        {error, Reason} ->
+            handle_child_exit(Pid, Reason, State)
+    end;
 handle_info(_Msg, State) ->
     %TODO: log unexpected message
     {noreply, State}.
 
 %% @hidden
 terminate(_Reason, #state{children = Children} = State) ->
-    RemainingChildren = loop_terminate(Children, []),
+    %% Shutdown children last to first.
+    RemainingChildren = loop_terminate(lists:reverse(Children), []),
     loop_wait_termination(RemainingChildren),
     {ok, State}.
 
@@ -284,34 +305,59 @@ terminate(_Reason, #state{children = Children} = State) ->
 handle_child_exit(Pid, Reason, State) ->
     case lists:keyfind(Pid, #child.pid, State#state.children) of
         false ->
-            {ok, State};
+            {noreply, State};
         #child{restart = {terminating, temporary, From}} ->
             gen_server:reply(From, ok),
             NewChildren = lists:keydelete(Pid, #child.pid, State#state.children),
-            {ok, State#state{children = NewChildren}};
+            {noreply, State#state{children = NewChildren}};
         #child{restart = {terminating, Restart, From}} = Child ->
             gen_server:reply(From, ok),
             NewChildren = lists:keyreplace(Pid, #child.pid, State#state.children, Child#child{
                 pid = undefined, restart = Restart
             }),
-            {ok, State#state{children = NewChildren}};
+            {noreply, State#state{children = NewChildren}};
         #child{} = Child ->
             case should_restart(Reason, Child#child.restart) of
                 true ->
-                    case try_start(Child) of
-                        {ok, NewPid, _Result} ->
-                            NewChild = Child#child{pid = NewPid},
-                            Children = lists:keyreplace(
-                                Pid, #child.pid, State#state.children, NewChild
-                            ),
-                            {ok, State#state{children = Children}}
-                    end;
+                    handle_restart_strategy(Child, State);
                 false ->
                     Children = lists:keydelete(Pid, #child.pid, State#state.children),
-                    {ok, State#state{children = Children}}
+                    {noreply, State#state{children = Children}}
             end
     end.
 
+handle_restart_strategy(
+    #child{id = Id} = Child, #state{restart_strategy = one_for_one} = State
+) ->
+    case try_start(Child) of
+        {ok, NewPid, _Result} ->
+            NewChild = Child#child{pid = NewPid},
+            Children = lists:keyreplace(
+                Id, #child.id, State#state.children, NewChild
+            ),
+            {noreply, State#state{children = Children}}
+    end;
+handle_restart_strategy(
+    #child{pid = Pid} = Child, #state{restart_strategy = one_for_all} = State
+) ->
+    Children =
+        case Pid of
+            {restarting, _} ->
+                State#state.children;
+            Pid when is_pid(Pid) ->
+                lists:keyreplace(Pid, #child.pid, State#state.children, Child#child{
+                    pid = {restarting, Pid}
+                })
+        end,
+    case terminate_one_for_all_restart(Children) of
+        {ok, NewChildren} ->
+            ok;
+        {ok, NewChildren, WaitExit} ->
+            ok = loop_wait_termination(WaitExit)
+    end,
+    {noreply, State#state{children = NewChildren},
+        {timeout, 0, {restart_many_children, NewChildren}}}.
+
 should_restart(_Reason, permanent) ->
     true;
 should_restart(_Reason, temporary) ->
@@ -367,6 +413,91 @@ try_start(#child{start = {M, F, Args}} = Record) ->
             {error, {{'EXIT', Error}, Record}}
     end.
 
+terminate_one_for_all_restart(Children) ->
+    %% Always shut down last child first
+    do_terminate_one_for_all_restart(lists:reverse(Children), [], []).
+
+do_terminate_one_for_all_restart([], NewChildren, []) ->
+    %% Do not reverse the list here, it was reversed before being accumulated
+    %% and is now in correct startup order.
+    {ok, NewChildren};
+do_terminate_one_for_all_restart([], NewChildren, WaitExit) ->
+    %% Do not reverse the list here, it was reversed before being accumulated
+    %% and is now in correct startup order.
+    {ok, NewChildren, WaitExit};
+do_terminate_one_for_all_restart([Child | Children], NewChildren, WaitExit) ->
+    case Child of
+        #child{restart = {terminating, temporary, From}} = Child when is_pid(From) ->
+            do_terminate(Child),
+            case verify_shutdown(Child) of
+                true ->
+                    do_terminate_one_for_all_restart(Children, NewChildren, WaitExit);
+                false ->
+                    do_terminate_one_for_all_restart(Children, NewChildren, [Child | WaitExit])
+            end;
+        #child{restart = {terminating, _Restart, From}} = Child when is_pid(From) ->
+            do_terminate(Child),
+            case verify_shutdown(Child) of
+                true ->
+                    do_terminate_one_for_all_restart(
+                        Children, [Child#child{pid = undefined} | NewChildren], WaitExit
+                    );
+                false ->
+                    do_terminate_one_for_all_restart(Children, [Child | NewChildren], [
+                        Child | WaitExit
+                    ])
+            end;
+        #child{pid = undefined, restart = temporary} = Child ->
+            do_terminate_one_for_all_restart(Children, NewChildren, WaitExit);
+        #child{pid = undefined} = Child ->
+            do_terminate_one_for_all_restart(Children, [Child | NewChildren], WaitExit);
+        #child{pid = {restarting, _Pid}} = Child ->
+            do_terminate_one_for_all_restart(Children, [Child | NewChildren], WaitExit);
+        #child{pid = Pid, restart = temporary} = Child when is_pid(Pid) ->
+            do_terminate(Child),
+            case verify_shutdown(Child) of
+                true ->
+                    do_terminate_one_for_all_restart(Children, NewChildren, WaitExit);
+                false ->
+                    do_terminate_one_for_all_restart(Children, NewChildren, [Child | WaitExit])
+            end;
+        #child{pid = Pid} = Child when is_pid(Pid) ->
+            do_terminate(Child),
+            case verify_shutdown(Child) of
+                true ->
+                    do_terminate_one_for_all_restart(
+                        Children, [Child#child{pid = {restarting, Pid}} | NewChildren], WaitExit
+                    );
+                false ->
+                    do_terminate_one_for_all_restart(
+                        Children, [Child#child{pid = {restarting, Pid}} | NewChildren], [
+                            Child | WaitExit
+                        ]
+                    )
+            end
+    end.
+
+verify_shutdown(#child{pid = Pid, shutdown = brutal_kill} = _Child) ->
+    receive
+        {'EXIT', Pid, _Reason} ->
+            true
+    after 100 ->
+        false
+    end;
+verify_shutdown(#child{pid = Pid, shutdown = Timeout} = _Child) ->
+    receive
+        {'EXIT', Pid, _Reason} ->
+            true
+    after Timeout ->
+        exit(Pid, kill),
+        receive
+            {'EXIT', Pid, killed} ->
+                true
+        after 100 ->
+            false
+        end
+    end.
+
 child_to_info(#child{id = Id, pid = Pid, type = Type, modules = Modules}) ->
     Child =
         case Pid of