From 098aa2ad54cfa81c07c49ed2a45d38e58bd3ecef Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Tue, 12 Apr 2011 16:47:17 +0200 Subject: Allow supervisor:terminate_child(SupRef,Pid) for simple_one_for_one supervisor:terminate_child/2 was not allowed if the supervisor used restart strategy simple_one_for_one. This is now changed so that children of this type of supervisors can be terminated by specifying the child's Pid. --- lib/stdlib/doc/src/supervisor.xml | 43 +++++++++++++++++--------- lib/stdlib/src/supervisor.erl | 58 ++++++++++++++++++++++++++---------- lib/stdlib/test/supervisor_SUITE.erl | 32 ++++++++++++++------ 3 files changed, 94 insertions(+), 39 deletions(-) diff --git a/lib/stdlib/doc/src/supervisor.xml b/lib/stdlib/doc/src/supervisor.xml index 45fa0847a8..d6203bdaa0 100644 --- a/lib/stdlib/doc/src/supervisor.xml +++ b/lib/stdlib/doc/src/supervisor.xml @@ -4,7 +4,7 @@
- 19962010 + 19962011 Ericsson AB. All Rights Reserved. @@ -83,11 +83,17 @@ supervisor, where all child processes are dynamically added instances of the same process type, i.e. running the same code.

-

The functions terminate_child/2, delete_child/2 +

The functions delete_child/2 and restart_child/2 are invalid for simple_one_for_one supervisors and will return {error,simple_one_for_one} if the specified supervisor uses this restart strategy.

+

The function terminate_child/2 can be used for + children under simple_one_for_one supervisors by + giving the child's pid() as the second argument. If + instead the child specification identifier is used, + terminate_child/2 will return + {error,simple_one_for_one}.

To prevent a supervisor from getting into an infinite loop of @@ -311,24 +317,33 @@ child_spec() = {Id,StartFunc,Restart,Shutdown,Type,Modules} SupRef = Name | {Name,Node} | {global,Name} | pid()  Name = Node = atom() - Id = term() + Id = pid() | term() Result = ok | {error,Error}  Error = not_found | simple_one_for_one -

Tells the supervisor SupRef to terminate the child - process corresponding to the child specification identified - by Id. The process, if there is one, is terminated but - the child specification is kept by the supervisor. This means - that the child process may be later be restarted by - the supervisor. The child process can also be restarted - explicitly by calling restart_child/2. Use - delete_child/2 to remove the child specification.

+

Tells the supervisor SupRef to terminate the given + child.

+

If the supervisor is not simple_one_for_one, + Id must be the child specification identifier. The + process, if there is one, is terminated but the child + specification is kept by the supervisor. The child process + may later be restarted by the supervisor. The child process + can also be restarted explicitly by calling + restart_child/2. Use delete_child/2 to remove + the child specification.

+

If the supervisor is simple_one_for_one, Id + must be the child process' pid(). I the specified + process is alive, but is not a child of the given + supervisor, the function will return + {error,not_found}. If the child specification + identifier is given instead instead of a pid(), the + function will return {error,simple_one_for_one}.

+

If successful, the function returns ok. If there is + no child specification with the specified Id, the + function returns {error,not_found}.

See start_child/2 for a description of SupRef.

-

If successful, the function returns ok. If there is - no child specification with the specified Id, - the function returns {error,not_found}.

diff --git a/lib/stdlib/src/supervisor.erl b/lib/stdlib/src/supervisor.erl index 368dc2e3e5..4fd7f1d47c 100644 --- a/lib/stdlib/src/supervisor.erl +++ b/lib/stdlib/src/supervisor.erl @@ -138,7 +138,7 @@ delete_child(Supervisor, Name) -> %%----------------------------------------------------------------- -type term_err() :: 'not_found' | 'simple_one_for_one'. --spec terminate_child(sup_ref(), term()) -> 'ok' | {'error', term_err()}. +-spec terminate_child(sup_ref(), pid() | term()) -> 'ok' | {'error', term_err()}. terminate_child(Supervisor, Name) -> call(Supervisor, {terminate_child, Name}). @@ -297,8 +297,26 @@ handle_call({start_child, EArgs}, _From, State) when ?is_simple(State) -> {reply, What, State} end; -%%% The requests terminate_child, delete_child and restart_child are -%%% invalid for simple_one_for_one supervisors. +%% terminate_child for simple_one_for_one can only be done with pid +handle_call({terminate_child, Name}, _From, State) when not is_pid(Name), + ?is_simple(State) -> + {reply, {error, simple_one_for_one}, State}; + +handle_call({terminate_child, Name}, _From, State) -> + case get_child(Name, State, ?is_simple(State)) of + {value, Child} -> + case do_terminate(Child, State#state.name) of + #child{restart_type=RT} when RT=:=temporary; ?is_simple(State) -> + {reply, ok, state_del_child(Child, State)}; + NChild -> + {reply, ok, replace_child(NChild, State)} + end; + false -> + {reply, {error, not_found}, State} + end; + +%%% The requests delete_child and restart_child are invalid for +%%% simple_one_for_one supervisors. handle_call({_Req, _Data}, _From, State) when ?is_simple(State) -> {reply, {error, simple_one_for_one}, State}; @@ -341,19 +359,6 @@ handle_call({delete_child, Name}, _From, State) -> {reply, {error, not_found}, State} end; -handle_call({terminate_child, Name}, _From, State) -> - case get_child(Name, State) of - {value, Child} -> - case do_terminate(Child, State#state.name) of - #child{restart_type = temporary} = NChild -> - {reply, ok, state_del_child(NChild, State)}; - NChild -> - {reply, ok, replace_child(NChild, State)} - end; - _ -> - {reply, {error, not_found}, State} - end; - handle_call(which_children, _From, #state{children = [#child{restart_type = temporary, child_type = CT, modules = Mods}]} = @@ -849,7 +854,28 @@ split_child(_, [], After) -> {lists:reverse(After), []}. get_child(Name, State) -> + get_child(Name, State, false). +get_child(Pid, State, AllowPid) when AllowPid, is_pid(Pid) -> + get_dynamic_child(Pid, State); +get_child(Name, State, _) -> lists:keysearch(Name, #child.name, State#state.children). + +get_dynamic_child(Pid, #state{children=[Child], dynamics=Dynamics}) -> + case is_dynamic_pid(Pid, dynamics_db(Child#child.restart_type, Dynamics)) of + true -> + {value, Child#child{pid=Pid}}; + false -> + case erlang:is_process_alive(Pid) of + true -> false; + false -> {value, Child} + end + end. + +is_dynamic_pid(Pid, Dynamics) when is_list(Dynamics) -> + lists:member(Pid, Dynamics); +is_dynamic_pid(Pid, Dynamics) -> + dict:is_key(Pid, Dynamics). + replace_child(Child, State) -> Chs = do_replace_child(Child, State#state.children), State#state{children = Chs}. diff --git a/lib/stdlib/test/supervisor_SUITE.erl b/lib/stdlib/test/supervisor_SUITE.erl index f9ceed8f84..cc271bd047 100644 --- a/lib/stdlib/test/supervisor_SUITE.erl +++ b/lib/stdlib/test/supervisor_SUITE.erl @@ -20,7 +20,7 @@ -module(supervisor_SUITE). --include_lib("test_server/include/test_server.hrl"). +-include_lib("common_test/include/ct.hrl"). -define(TIMEOUT, 1000). %% Testserver specific export @@ -349,8 +349,7 @@ child_adm(Config) when is_list(Config) -> ok = supervisor:terminate_child(sup_test, child1), %% Start of already existing but not running process - {error,already_present} = - supervisor:start_child(sup_test, Child), + {error,already_present} = supervisor:start_child(sup_test, Child), %% Restart {ok, CPid2} = supervisor:restart_child(sup_test, child1), @@ -377,6 +376,11 @@ child_adm(Config) when is_list(Config) -> [{child1, CPid3, worker, []}] = supervisor:which_children(sup_test), [1,1,0,1] = get_child_counts(sup_test), + %% Terminate with Pid not allowed when not simple_one_for_one + {error,not_found} = supervisor:terminate_child(sup_test, CPid3), + [{child1, CPid3, worker, []}] = supervisor:which_children(sup_test), + [1,1,0,1] = get_child_counts(sup_test), + {'EXIT',{noproc,{gen_server,call,[foo,which_children,infinity]}}} = (catch supervisor:which_children(foo)), {'EXIT',{noproc,{gen_server,call,[foo,count_children,infinity]}}} @@ -412,16 +416,26 @@ child_adm_simple(Config) when is_list(Config) -> [1,2,0,2] = get_child_counts(sup_test), %% Termination - {error, simple_one_for_one} = - supervisor:terminate_child(sup_test, child1), + {error, simple_one_for_one} = supervisor:terminate_child(sup_test, child1), + [1,2,0,2] = get_child_counts(sup_test), + ok = supervisor:terminate_child(sup_test,CPid1), + [_] = supervisor:which_children(sup_test), + [1,1,0,1] = get_child_counts(sup_test), + false = erlang:is_process_alive(CPid1), + %% Terminate non-existing proccess is ok + ok = supervisor:terminate_child(sup_test,CPid1), + [_] = supervisor:which_children(sup_test), + [1,1,0,1] = get_child_counts(sup_test), + %% Terminate pid which is not a child of this supervisor is not ok + NoChildPid = spawn_link(fun() -> receive after infinity -> ok end end), + {error, not_found} = supervisor:terminate_child(sup_test, NoChildPid), + true = erlang:is_process_alive(NoChildPid), %% Restart - {error, simple_one_for_one} = - supervisor:restart_child(sup_test, child1), + {error, simple_one_for_one} = supervisor:restart_child(sup_test, child1), %% Deletion - {error, simple_one_for_one} = - supervisor:delete_child(sup_test, child1), + {error, simple_one_for_one} = supervisor:delete_child(sup_test, child1), ok. %%------------------------------------------------------------------------- -- cgit v1.2.3 From 1a8341595c1bca1f46aa0afee8e5f01675379980 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Wed, 13 Apr 2011 15:32:55 +0200 Subject: Add terminate_child(Sup, Pid) for simple_one_for_one --- system/doc/design_principles/sup_princ.xml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/system/doc/design_principles/sup_princ.xml b/system/doc/design_principles/sup_princ.xml index 067fd31961..2748f21bbe 100644 --- a/system/doc/design_principles/sup_princ.xml +++ b/system/doc/design_principles/sup_princ.xml @@ -4,7 +4,7 @@
- 19972009 + 19972011 Ericsson AB. All Rights Reserved. @@ -335,6 +335,12 @@ supervisor:start_child(Pid, [id1]) apply(call, start_link, []++[id1]), or actually:

call:start_link(id1) +

A child under a simple_one_for_one supervisor can be terminated + with

+ +supervisor:terminate_child(Sup, Pid) +

where Sup is the pid, or name, of the supervisor and + Pid is the pid of the child.

-- cgit v1.2.3