[erlang-patches] Supervisor Shutdown

[Siri Hansen]

Hi Vance,

a while back (I think R13B), there was a change in stdlib (from the release
note):

"When a process started with proc_lib, gen_server, or gen_fsm exits with
reason {shutdown,Term}, a crash report will no longer be generated (to allow
a clean shutdown, but still provide additional information to process that
are linked to the terminating process)."

I think - what causes your particular problem in radius is that supervisor
is not adapted to this change. I.e. even though there will be no error- or
crash report from the child process (gen_fsm) itself, there will still be a
supervisor report.

We find it fair to correct this! However, this change will not be fully
backwards compatible because removing the supervisor report is not enough -
we must also remove the supervisor's attempt at restarting the child
(depening on the child's restart type). This means that we can not do this
in a patch release - we must wait for R15.


When it comes to {normal,Info} we have decided not to implement that since
it is not obvious how it should work (if it should be equivalent to normal
(which has a special meaning in the language) or as shutdown (which really
only has a special meaning in OTP/supervisor/proc_lib)).


I hope this is an ok solution for you? I will do the correction for R15.

Regards
/siri





2011/4/13 Vance Shipley <vances@REDACTED>

> Siri et. al.,
>
> Thank you, I'm glad there is agreement that the functionality is desired.
>
> In a similiar vein I'd like to see an enhancement to allow passing
> information back in the exit message of a child.  Currently a child's
> init/1 function may return {ok,Child} or {ok,Child,Info} and
> supervisor:start_child/2 returns the same.  I'd like to see the inverse
> where the child may exit with either 'shutdown' or {shutdown, Info},
> in response to a shutdown request from it's supervisor, and 'normal'
> or {normal, Info} for a normal exit.
>
> The child may exit with any value now of course however if the exit
> value isn't expected the supervisor restart strategy and SASL error
> logging will not work as desired.
>
> You may look to my radierl(*) project for an example of a practical
> application of this functionality.  In this stack application the
> radius_server module implements a gen_server which will dynamically
> start a new gen_fsm process, under supervision, to service requests.
> It keeps the gen_fsm's pid in state stored in a gb_tree() with the
> client's info as the key.  In this way client requests can be quickly
> looked up and forwarded to the gen_fsm.  When the gen_fsm terminates
> the gen_server needs to reap the state.
>
> I implemented a clause in radius_server to efficiently do this:
>
>        handle_info({'EXIT', _Pid, {shutdown, Key}},
>                #state{handlers = Handlers} = State) ->
>            NewHandlers = gb_trees:delete(Key, Handlers),
>            NewState = State#state{handlers = NewHandlers},
>            {noreply, NewState};
>
> However I can't use this because SASL reports are generated for normal
> terminations.  Instead I have to handle the normal expected termination
> case in the same way as the unexpected case (i.e. inefficiently):
>
>        handle_info({'EXIT', Fsm, _Reason},
>              #state{handlers = Handlers} = State) ->
>           Fdel = fun(_F, {Key, Pid, _Iter}) when Pid == Fsm ->
>                    Key;
>                 (F, {_Key, _Val, Iter}) ->
>                    F(F, gb_trees:next(Iter));
>                 (_F, none) ->
>                    none
>           end,
>           Iter = gb_trees:iterator(Handlers),
>           case Fdel(Fdel, gb_trees:next(Iter)) of
>              none ->
>                 {noreply, State};
>              Key ->
>                 NewHandlers = gb_trees:delete(Key, Handlers),
>                 NewState = State#state{handlers = NewHandlers},
>                 {noreply, NewState}
>           end.
>
> I would be willing to implement a patch if the team agrees with the
> approach.
>
>        -Vance
>
> (*)  http://github.com/vances/radierl
>      http://motivity.ca/radius
>
> On Wed, Apr 06, 2011 at 10:40:56AM +0200, Siri Hansen wrote:
> }  We have now discussed this matter and decided that instead of adding a
> new
> }  terminate/1 function we will add a clause to terminate_child/2 and allow
> the
> }  child to be pointed out by Pid instead of Id.
> }
> }  The correction will be included in R14B03.
> }
> }  Thank you for the contribution!
> }
> }  Best regards
> }  /siri
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20110413/24b58c90/attachment.htm>