[erlang-bugs 9] Re: possible supervisor bug in r14b02

[Filipe David Manana]

Thanks :)

On Mon, Mar 28, 2011 at 9:24 AM, Ingela Anderton Andin
<ingela@REDACTED> wrote:
> Hi!
>
> We will take a look at your patch it sounds like it is the right thing to
> do.
> Temporary processes should not be restarted so you should not have
> to save their  init-arguments, although until the last release they where
> saved
> so it was possible to restart them! Especially for simple_one_for_one
> supervisors
> that may have lots of temporary processes memory consumption can
> go sky high if you save them.
>
> Regards  Ingela Erlang OTP team - Ericsson AB
>
> Filipe David Manana wrote:
>>
>> Hi,
>>
>> In R14B02 I noticed that for a child with a "temporary" restart_type,
>> we discard its A component of the MFA tuple when adding the childspec
>> to the list of the supervisor's children [1].
>>
>> When the child terminates, its spec is never removed from the list of
>> the supervisor's children specs.
>> Then if we call supervisor:restart_child/2 after the child terminates,
>> the handle_call clause for restart_child gets the childspec with an
>> MFA  that is {M, F, undefined} [2]. At that point do_start_child will
>> call apply(M, F, undefined) [3] which will cause the supervisor to
>> reply with an error, instead of returning {ok, Pid} as in previous
>> releases. An example for the returned error:
>>
>>  {error,{'EXIT',{badarg,[{erlang,apply,[gen_server,start_link,undefined]},
>>                              {supervisor,do_start_child,2},
>>                              {supervisor,handle_call,3},
>>                              {gen_server,handle_msg,5},
>>                              {proc_lib,init_p_do_apply,3}]}}}
>>
>> The patch at [4] fixes the issue for me. The particular code that is
>> no longer working in R14B02 but worked on all previous releases, is
>> from Apache CouchDB, see [5]
>>
>> This issue was introuced by OTP-9064 (reading from the R14B02 release
>> notes).
>> Was this intended behaviour? It doesn't make much sense for me to keep
>> a temporary childspec in the supervisor once the child terminates, so
>> I believe deleting it from the state is the right thing to do.
>>
>> [1] -
>> https://github.com/erlang/otp/blob/dev/lib/stdlib/src/supervisor.erl#L787
>> [2] -
>> https://github.com/erlang/otp/blob/dev/lib/stdlib/src/supervisor.erl#L314
>> [3] -
>> https://github.com/erlang/otp/blob/dev/lib/stdlib/src/supervisor.erl#L246
>> [4] -
>> https://github.com/fdmanana/otp/commit/2697042aa9ebab2fcd208c93b7f454b25bc580d4
>> [5] -
>> https://github.com/apache/couchdb/blob/trunk/src/couchdb/couch_replicator.erl#L119
>>
>>
>
> _______________________________________________
> erlang-bugs mailing list
> erlang-bugs@REDACTED
> http://erlang.org/mailman/listinfo/erlang-bugs
>



-- 
Filipe David Manana,
fdmanana@REDACTED, fdmanana@REDACTED

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."