[erlang-questions] More on this: simple_one_by_one trouble and rare start_link/2

Angel J. Alvarez Miguel clist@REDACTED
Thu Jun 28 10:10:34 CEST 2012


How much is used this feature? How many people use it ?

I dont think so many, after  a couple of greps along my installed OTP packages i would say i dont think anyone  is using 
this simple_one_for_one feaure as all MFA's in child specs seem to have the A part as [ ] so the args count in fixed by what you 
put on start_child/2. The best i cant say in my limited knowledge is that this feature is nullified buy this fact so apparently almost 
nobody is using it, hence the dumb patch i did.

However i didnt meant having all callbacks as init/1 but in this case provided that start_child can have arbitrary args list 
this would make your posible callbacks arity any, at any time, upon what you put on the args. 

this is error prone at least 

In the best case scenario you can pass both args surrounded in list so you get a Module:init/2 callback to be called.... better
 than nothing.

Do in the end maybe it is worth check if spec args are [] (the usual case) and keep doing a simple ++ but in other case do a better list joint
and keep your callback arity sane as documented...

I think it deserve at least be documented so other guys dont get stuck with this on the future .


/Angel

On Miércoles, 27 de Junio de 2012 14:41:58 usted escribió:
> I don't really think that there is an inconsistency here.
> 
> In all the cases where something is to be started, supervisor, application
> and proc_lib, the specifier for this is the Mod,Fun,Args of apply and
> spawn. Though I will admit that in the application case the function name
> is predefined, almost like a call back function. And for callbacks all the
> arguments, number and meaning, are predefined.
> 
> I personally think the main problems are:
> 
> - Inconsistency in the documentation caused by using "Args" to mean
> different things. So in supervisor. application and proc_lib "Args" really
> is a list of arguments in apply/spawn style where the call is built using
> the length of the list to determine the arity of the called function. In
> gen_XXX the ONE argument to Mod:init/1 is also called "Args" where it can
> be anything and even if it is a list it will only become one argument.
> Calling it "Data" would have made it easier to understand.
> 
> - The second argument supervisor:start_child/2 to completely different,
> both type and meaning, depending on whether the supervisor is a
> simple_one_for_one or anything else. Yes, I know that it is documented but
> it IS inconsistent.
> 
> I think that in general making all the start functions arity 1 and calling
> them with a list of arguments is of no real benefit. Apart from
> simple_one_for_one supervisors the arguments to the start function are
> anyway fixed so there would be no real gain here anyway.
> 
> Also it would be a completely backwards incompatible change which would
> cause a lot of rewriting, so I don't see it being applied.
> 
> We will just have to live with it. If it is of any consolation there are
> changes I would like to make which would also never be made. :-)
> 
> Robert

Ok ok, at least im not completely wrong, ill try to overcome this with other 
approach like passing a pid of something else so children can ask some adult 
whatever they know about their new playground. 

Maybe things like this deserve a place in a TODO list for a future rewrite or 
a OTP next-generation saga....

As always the good side of the history is a better understanding on the inner 
working of the OTP beats.

Thanks Angel
> 
> ----- Original Message -----
> 
> > 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,[arg
> > 1,arg2,arg3,arg4],[]},{supervisor,do_start_child_i,3,[{file,"supervisor.e
> > rl"},{line,319}]},{supervisor,handle_call,3,[{file,"supervisor.erl"},{lin
> > e,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



More information about the erlang-questions mailing list