[erlang-questions] More on this: simple_one_by_one trouble and rare start_link/2
Siri Hansen
erlangsiri@REDACTED
Tue Jul 3 10:01:50 CEST 2012
Hi Angel!
Thanks for pointing out this problem!
While one might agree that the design decision was not very good here, it
is, as Robert points out, a totally backwards incompatible change you wish
to do. And even though simple_one_for_one is not the most used supervisor
type, it is for sure used and it is not an option for us to do such a
change.
However, I do agree that the documentation could be better here, and I will
write a ticket to make sure the issue is not forgotten. As always, a user
contribution might very well make this change happen faster than our own
priorities else would allow :)
Best regards
/siri@REDACTED
2012/6/22 Angel J. Alvarez Miguel <clist@REDACTED>
>
> i did'nt find any example of mixing args from the supervirsor childspec
> and start_child(...) so im going to make my own digging in this issue:
>
> Let see How the simple_one_by_one works...
>
> handle_call({start_child, EArgs}, _From, State) when ?is_simple(State) ->
> Child = hd(State#state.children),
> #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};
> <------>{ok, Pid, Extra} ->
> <------> NState = save_dynamic_child(Child#child.restart_type, Pid,
> Args, State),
> <------> {reply, {ok, Pid, Extra}, NState};
> <------>What ->
> <------> {reply, What, State}
> end;
>
>
> Shouldnt be do_start_child_i(M,F,EArgs) of the form :
>
> do_start_child(M,F,[EArgs]) ??
>
> or the inner call apply(M,F,[A])?
>
> Provided the docs state that your unique exported funcion must be of
> arity one you should
> expect that code will take measures in order to enforce arity one,
> enclosing whatever you pass it in a List on the apply phase.
>
> The semanctics of apply use a list to KNOW how many args the target call
> has, while the
> semantics of sup childspec + sup start_child use a list ++ op to know how
> many args will be passed
> to your callback of arity ONE.
>
> So for the apply part all your functions are arity ONE
>
> Without the ability to join Args from the ChildSpec and from the start
> child call i would concur that you should
> enclose in a list whatever you want to be as a sole argument and this code
> be ok, but as soon as you can put
> two or more args one on the ChildSpec and one on the call Sup code must
> enforce that only one arg APPLY
> call is made so you end calling start_link/1 whatever you pass on the
> args...
>
> Even the DOCS state "[ term() ]" as the MFA on the ChildSpec
>
>
> My stdlib-1.18.1.pdf says (Pag 369)
>
> "start_child(SupRef, ChildSpec) -> startchild_ret()
> Types:
> SupRef = sup_ref()
> ChildSpec = child_spec() | (List :: [term()])
> ...
> ...
>
> ...If the case of a simple_one_for_one supervisor, the child specification
> defined in Module:init/1 will be
> used and ChildSpec should instead be an arbitrary list of terms List. The
> child process will then be started by
> appending List to the existing start function arguments, i.e. by calling
> apply(M, F, A++List) where {M,F,A}
> is the start function defined in the child specification."
>
> Here IMHO lies the error, as A is a list we have [any()...] ++ [any()...]
> is a list [any()...,any()...] and thats get us an apply
> call of arity > 1 ALWAYS even when you chilkdspec has a {M,F,[]} and you
> try provide a [..] in your start_child it will fail
> unless both argslists are void.
>
> So a combination of:
>
> init(Opts) ->
> io:format("[GROUP SUP] Ready to manage MuC with opts: ~p\'s\n",[Opts]),
>
> {ok, {
> {simple_one_for_one, 1, 60},
> [
> {mucfsm, {sim_group_fsm, start_link, [arg1,arg2]},
> transient, 60, worker, [sim_group]}
> ]
> }
> }.
>
>
> and a client that makes a call like:
> ...
> {ok,ChildPid} = supervisor:start_child(SupPid,[arg3,arg4]),
> ...
>
> Gets you a final apply call {M,F,A} where A is in the form
> [arg1,arg2,arg3,arg4]
> not [[arg1,arg2,argf3,arg4]] at is should be when you intent to call a
> start_link/1....
>
>
>
> Erlang R15B01 (erts-5.9.1) [source] [64-bit] [smp:4:4] [async-threads:0]
> [hipe] [kernel-poll:false]
>
> [QuickCheck] [PropEr] [Erl0MQ]
> Starting simple_one_by_one group supervisor...
> [GROUP SUP] staring with argument: [{option1,40}]
> Eshell V5.9.1 (abort with ^G)
> 1> [GROUP SUP] Ready to manage MuC with opts: [{option1,40}]'s
> Adding a child dynamically...
> {"init terminating in
> do_boot",{{badmatch,{error,{'EXIT',{undef,[{sim_group_fsm,start_link,[arg1,arg2,arg3,arg4],[]},{supervisor,do_start_child_i,3,[{file,"supervisor.erl"},{line,319}]},{supervisor,handle_call,3,[{file,"supervisor.erl"},{line,344}]},
>
> {gen_server,handle_msg,5,[{file,"gen_server.erl"},{line,588}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,227}]}]}}}},[{test,main,0,[{file,"test.erl"},{line,11}]},{init,start_it,1,[]},{init,start_em,1,[]}]}}
>
>
> /Angel
>
> On Jueves, 21 de Junio de 2012 09:30:15 Angel J. Alvarez Miguel escribió:
> > That's right...
> >
> > But start_link it is expected to be or arity 3 and Opts is a property
> list.
> >
> > but then on test:main/0 when i do supervisor:start_child(SupPid,Opts2)
> >
> > being Opts2 another property list, then suppervisor should call:
> >
> > apply(sim_group_fsm,start_link,Opts ++ Opts2) on its line 319 which in
> turn
> > calls gen_fsm:start_link, thus providing a Pid in the end...
> >
> > but what it really does is apply(sim_group_fsm,start_link,Opts ++ Opts2,[
> > ]) and then crashes unless i declare a start_link/2 on my gen_fsm...
> >
> > {
> > {badmatch,
> > {error,
> > {'EXIT',
> > {undef,
> > [
> >
> {sim_group_fsm,start_link,[[{option1,40}],[{option29,100}]],[]},
> > {supervisor,do_start_child_i,3,[
> >
> {file,"supervisor.erl"},
> >
> {line,319}
> >
> ]}......
> >
> > i dont see why i finish having a apply/4 call in supervisor: 319 instead
> of
> > an apply/3...
> >
> > I want the sup to propagate one property list to children specs while i
> > will provide another during children spawns providing that sup combines
> > two in any way children will manage to demunge ...both property lists
> >
> > So cant just see what im doing wrong....
> >
> > /angel
> >
> >
> > PS: i changed a bit the test code to force sup crash
> >
> > On Jueves, 21 de Junio de 2012 06:22:54 Vance Shipley escribió:
> > > On Thu, Jun 21, 2012 at 12:30:08AM +0200, Angel J. Alvarez Miguel
> wrote:
> > > } While calling a simple_one_by_one supervisor to start a gen fsm I
> > > found } it causes calling a start_link/2 function on the callback
> > > module of the } gen_fsm.
> > >
> > > You are specifying that function to be called to start the worker
> > > yourself. In your module group_supervisor.erl you provide a
> child_spec()
> > > in the
> > >
> > > return value of init/1:
> > > {mucfsm, {sim_group_fsm, start_link, [Opts]}, transient, 60, worker,
> > >
> > > [sim_group]}
> > >
> > > The form of which is:
> > > {Id, StartFunc, Restart, Shutdown, Type, Modules}
> > >
> > > The second argument, StartFunc, has the form:
> > > {Module, Function, Arguments}
> > >
> > > You provided:
> > > {sim_group_fsm, start_link, [Opts]}
> _______________________________________________
> erlang-questions mailing list
> erlang-questions@REDACTED
> http://erlang.org/mailman/listinfo/erlang-questions
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-questions/attachments/20120703/5a40fe67/attachment.htm>
More information about the erlang-questions
mailing list