From c2b6342cb57df5925acbacff0aea74358c9a5ffb Mon Sep 17 00:00:00 2001 From: Tomas Pihl Date: Sun, 22 Jul 2012 13:01:43 +0200 Subject: Have supervisor send errors up the chain If a child fails to start, supervisor relies upon error_logger which does not work when IO is inhibited. Instead pass the error up the chain and let someone else use a proper Reason for any possible printouts. --- lib/stdlib/doc/src/supervisor.xml | 2 +- lib/stdlib/src/supervisor.erl | 24 +++++++++------- lib/stdlib/test/supervisor_SUITE.erl | 30 +++++++++++++++++++- lib/stdlib/test/supervisor_SUITE_data/Makefile.src | 15 ++++++++++ .../app_faulty/ebin/app_faulty.app | 10 +++++++ .../app_faulty/src/app_faulty.erl | 17 ++++++++++++ .../app_faulty/src/app_faulty_server.erl | 32 ++++++++++++++++++++++ .../app_faulty/src/app_faulty_sup.erl | 17 ++++++++++++ 8 files changed, 135 insertions(+), 12 deletions(-) create mode 100644 lib/stdlib/test/supervisor_SUITE_data/Makefile.src create mode 100644 lib/stdlib/test/supervisor_SUITE_data/app_faulty/ebin/app_faulty.app create mode 100644 lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty.erl create mode 100644 lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty_server.erl create mode 100644 lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty_sup.erl (limited to 'lib/stdlib') diff --git a/lib/stdlib/doc/src/supervisor.xml b/lib/stdlib/doc/src/supervisor.xml index f9a5e245b4..1a64b7a2af 100644 --- a/lib/stdlib/doc/src/supervisor.xml +++ b/lib/stdlib/doc/src/supervisor.xml @@ -295,7 +295,7 @@ child_spec() = {Id,StartFunc,Restart,Shutdown,Type,Modules} terminates with reason Term.

If any child process start function fails or returns an error tuple or an erroneous value, the function returns - {error,shutdown} and the supervisor terminates all + {error, {shutdown, Reason}} and the supervisor terminates all started child processes and then itself with reason shutdown.

diff --git a/lib/stdlib/src/supervisor.erl b/lib/stdlib/src/supervisor.erl index 7d3c5a0e21..7624c6cd18 100644 --- a/lib/stdlib/src/supervisor.erl +++ b/lib/stdlib/src/supervisor.erl @@ -104,7 +104,9 @@ %%% SupName = {local, atom()} | {global, atom()}. %%% --------------------------------------------------- --type startlink_err() :: {'already_started', pid()} | 'shutdown' | term(). +-type startlink_err() :: {'already_started', pid()} + | {'shutdown', term()} + | term(). -type startlink_ret() :: {'ok', pid()} | 'ignore' | {'error', startlink_err()}. -spec start_link(Module, Args) -> startlink_ret() when @@ -221,8 +223,10 @@ cast(Supervisor, Req) -> -type init_sup_name() :: sup_name() | 'self'. --type stop_rsn() :: 'shutdown' | {'bad_return', {module(),'init', term()}} - | {'bad_start_spec', term()} | {'start_spec', term()} +-type stop_rsn() :: {'shutdown', term()} + | {'bad_return', {module(),'init', term()}} + | {'bad_start_spec', term()} + | {'start_spec', term()} | {'supervisor_data', term()}. -spec init({init_sup_name(), module(), [term()]}) -> @@ -253,9 +257,9 @@ init_children(State, StartSpec) -> case start_children(Children, SupName) of {ok, NChildren} -> {ok, State#state{children = NChildren}}; - {error, NChildren} -> + {error, NChildren, Reason} -> terminate_children(NChildren, SupName), - {stop, shutdown} + {stop, {shutdown, Reason}} end; Error -> {stop, {start_spec, Error}} @@ -275,9 +279,9 @@ init_dynamic(_State, StartSpec) -> %% Func: start_children/2 %% Args: Children = [child_rec()] in start order %% SupName = {local, atom()} | {global, atom()} | {pid(), Mod} -%% Purpose: Start all children. The new list contains #child's +%% Purpose: Start all children. The new list contains #child's %% with pids. -%% Returns: {ok, NChildren} | {error, NChildren} +%% Returns: {ok, NChildren} | {error, NChildren, Reason} %% NChildren = [child_rec()] in termination order (reversed %% start order) %%----------------------------------------------------------------- @@ -293,7 +297,7 @@ start_children([Child|Chs], NChildren, SupName) -> start_children(Chs, [Child#child{pid = Pid}|NChildren], SupName); {error, Reason} -> report_error(start_error, Reason, Child, SupName), - {error, lists:reverse(Chs) ++ [Child | NChildren]} + {error, lists:reverse(Chs) ++ [Child | NChildren], Reason} end; start_children([], NChildren, _SupName) -> {ok, NChildren}. @@ -793,7 +797,7 @@ restart(rest_for_one, Child, State) -> case start_children(ChAfter2, State#state.name) of {ok, ChAfter3} -> {ok, State#state{children = ChAfter3 ++ ChBefore}}; - {error, ChAfter3} -> + {error, ChAfter3, _Reason} -> NChild = Child#child{pid=restarting(Child#child.pid)}, NState = State#state{children = ChAfter3 ++ ChBefore}, {try_again, replace_child(NChild,NState)} @@ -804,7 +808,7 @@ restart(one_for_all, Child, State) -> case start_children(Children2, State#state.name) of {ok, NChs} -> {ok, State#state{children = NChs}}; - {error, NChs} -> + {error, NChs, _Reason} -> NChild = Child#child{pid=restarting(Child#child.pid)}, NState = State#state{children = NChs}, {try_again, replace_child(NChild,NState)} diff --git a/lib/stdlib/test/supervisor_SUITE.erl b/lib/stdlib/test/supervisor_SUITE.erl index 767ae3d62c..d9f2e82b31 100644 --- a/lib/stdlib/test/supervisor_SUITE.erl +++ b/lib/stdlib/test/supervisor_SUITE.erl @@ -46,6 +46,7 @@ temporary_normal/1, permanent_shutdown/1, transient_shutdown/1, temporary_shutdown/1, + faulty_application_shutdown/1, permanent_abnormal/1, transient_abnormal/1, temporary_abnormal/1, temporary_bystander/1]). @@ -98,7 +99,8 @@ groups() -> {normal_termination, [], [permanent_normal, transient_normal, temporary_normal]}, {shutdown_termination, [], - [permanent_shutdown, transient_shutdown, temporary_shutdown]}, + [permanent_shutdown, transient_shutdown, temporary_shutdown, + faulty_application_shutdown]}, {abnormal_termination, [], [permanent_abnormal, transient_abnormal, temporary_abnormal]}, @@ -658,6 +660,32 @@ temporary_shutdown(Config) when is_list(Config) -> [] = supervisor:which_children(sup_test), [0,0,0,0] = get_child_counts(sup_test). +%%------------------------------------------------------------------------- +%% Faulty application should shutdown and pass on errors +faulty_application_shutdown(Config) when is_list(Config) -> + + %% Set some paths + AppDir = filename:join(?config(data_dir, Config), "app_faulty"), + EbinDir = filename:join(AppDir, "ebin"), + + %% Start faulty app + code:add_patha(EbinDir), + + % {error, + % {{shutdown, + % {undef,[ + % {an_undefined_module_with,an_undefined_function,[argument1,argument2],[]}, + % {app_faulty_server,init,1,[{file,"app_faulty/src/app_faulty_server.erl"},{line,16}]}, + % {gen_server,init_it,6,[{file,"gen_server.erl"},{line,304}]}, + % {proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,227}]} + % ]}},{app_faulty,start,[normal,[]]}}} + + {error, Error} = application:start(app_faulty), + {{shutdown, {undef, CallStack}},{app_faulty,start,_}} = Error, + [{an_undefined_module_with,an_undefined_function,_,_}|_] = CallStack, + ok = application:unload(app_faulty), + ok. + %%------------------------------------------------------------------------- %% A permanent child should always be restarted. permanent_abnormal(Config) when is_list(Config) -> diff --git a/lib/stdlib/test/supervisor_SUITE_data/Makefile.src b/lib/stdlib/test/supervisor_SUITE_data/Makefile.src new file mode 100644 index 0000000000..dbc5729f47 --- /dev/null +++ b/lib/stdlib/test/supervisor_SUITE_data/Makefile.src @@ -0,0 +1,15 @@ +EFLAGS=+debug_info + +APP_FAULTY= \ + app_faulty/ebin/app_faulty_sup.@EMULATOR@ \ + app_faulty/ebin/app_faulty_server.@EMULATOR@ \ + app_faulty/ebin/app_faulty.@EMULATOR@ \ + +all: $(APP_FAULTY) + +app_faulty/ebin/app_faulty_server.@EMULATOR@: app_faulty/src/app_faulty_server.erl + erlc $(EFLAGS) -oapp_faulty/ebin app_faulty/src/app_faulty_server.erl +app_faulty/ebin/app_faulty_sup.@EMULATOR@: app_faulty/src/app_faulty_sup.erl + erlc $(EFLAGS) -oapp_faulty/ebin app_faulty/src/app_faulty_sup.erl +app_faulty/ebin/app_faulty.@EMULATOR@: app_faulty/src/app_faulty.erl + erlc $(EFLAGS) -oapp_faulty/ebin app_faulty/src/app_faulty.erl diff --git a/lib/stdlib/test/supervisor_SUITE_data/app_faulty/ebin/app_faulty.app b/lib/stdlib/test/supervisor_SUITE_data/app_faulty/ebin/app_faulty.app new file mode 100644 index 0000000000..d4ab07e485 --- /dev/null +++ b/lib/stdlib/test/supervisor_SUITE_data/app_faulty/ebin/app_faulty.app @@ -0,0 +1,10 @@ +{application, app_faulty, + [{description, "very simple example faulty application"}, + {id, "app_faulty"}, + {vsn, "1.0"}, + {modules, [app_faulty, app_faulty_sup, app_faulty_server]}, + {registered, [app_faulty]}, + {applications, [kernel, stdlib]}, + {env, [{var,val1}]}, + {mod, {app_faulty, []}} + ]}. diff --git a/lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty.erl b/lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty.erl new file mode 100644 index 0000000000..c65b411cd6 --- /dev/null +++ b/lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty.erl @@ -0,0 +1,17 @@ +-module(app_faulty). + +-behaviour(application). + +%% Application callbacks +-export([start/2, stop/1]). + +start(_Type, _StartArgs) -> + case app_faulty_sup:start_link() of + {ok, Pid} -> + {ok, Pid}; + Error -> + Error + end. + +stop(_State) -> + ok. diff --git a/lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty_server.erl b/lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty_server.erl new file mode 100644 index 0000000000..6628f92210 --- /dev/null +++ b/lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty_server.erl @@ -0,0 +1,32 @@ +-module(app_faulty_server). + +-behaviour(gen_server). + +%% API +-export([start_link/0]). + +%% gen_server callbacks +-export([init/1, handle_call/3, handle_cast/2, handle_info/2, + terminate/2, code_change/3]). + +start_link() -> + gen_server:start_link({local, ?MODULE}, ?MODULE, [], []). + +init([]) -> + an_undefined_module_with:an_undefined_function(argument1, argument2), + {ok, []}. + +handle_call(_Request, _From, State) -> + {reply, ok, State}. + +handle_cast(_Msg, State) -> + {noreply, State}. + +handle_info(_Info, State) -> + {noreply, State}. + +terminate(_Reason, _State) -> + ok. + +code_change(_OldVsn, State, _Extra) -> + {ok, State}. diff --git a/lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty_sup.erl b/lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty_sup.erl new file mode 100644 index 0000000000..8115a88809 --- /dev/null +++ b/lib/stdlib/test/supervisor_SUITE_data/app_faulty/src/app_faulty_sup.erl @@ -0,0 +1,17 @@ +-module(app_faulty_sup). + +-behaviour(supervisor). + +%% API +-export([start_link/0]). + +%% Supervisor callbacks +-export([init/1]). + +start_link() -> + supervisor:start_link(?MODULE, []). + +init([]) -> + AChild = {app_faulty,{app_faulty_server,start_link,[]}, + permanent,2000,worker,[app_faulty_server]}, + {ok,{{one_for_all,0,1}, [AChild]}}. -- cgit v1.2.3 From 102adf32f766dceee6f277c34111fbe9b8971647 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Thu, 11 Oct 2012 11:07:29 +0200 Subject: Fix documentation about how supervisor handles crash in child's start function --- lib/stdlib/doc/src/supervisor.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'lib/stdlib') diff --git a/lib/stdlib/doc/src/supervisor.xml b/lib/stdlib/doc/src/supervisor.xml index 1a64b7a2af..9021d02ade 100644 --- a/lib/stdlib/doc/src/supervisor.xml +++ b/lib/stdlib/doc/src/supervisor.xml @@ -294,10 +294,10 @@ child_spec() = {Id,StartFunc,Restart,Shutdown,Type,Modules} is a term with information about the error, and the supervisor terminates with reason Term.

If any child process start function fails or returns an error - tuple or an erroneous value, the function returns - {error, {shutdown, Reason}} and the supervisor terminates all - started child processes and then itself with reason - shutdown.

+ tuple or an erroneous value, the supervisor will first terminate + all already started child processes with reason shutdown + and then terminate itself and return + {error, {shutdown, Reason}}.

-- cgit v1.2.3 From 95af7e7bec94d9ee2ba7cbbe4aa045862ddcd386 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Thu, 11 Oct 2012 11:50:40 +0200 Subject: If supervisor:start_link fails to start child, add child id to error reason --- lib/stdlib/src/supervisor.erl | 3 ++- lib/stdlib/test/supervisor_SUITE.erl | 27 +++++++++++++++++---------- 2 files changed, 19 insertions(+), 11 deletions(-) (limited to 'lib/stdlib') diff --git a/lib/stdlib/src/supervisor.erl b/lib/stdlib/src/supervisor.erl index 7624c6cd18..9f93747c3e 100644 --- a/lib/stdlib/src/supervisor.erl +++ b/lib/stdlib/src/supervisor.erl @@ -297,7 +297,8 @@ start_children([Child|Chs], NChildren, SupName) -> start_children(Chs, [Child#child{pid = Pid}|NChildren], SupName); {error, Reason} -> report_error(start_error, Reason, Child, SupName), - {error, lists:reverse(Chs) ++ [Child | NChildren], Reason} + {error, lists:reverse(Chs) ++ [Child | NChildren], + {failed_to_start_child,Child#child.name,Reason}} end; start_children([], NChildren, _SupName) -> {ok, NChildren}. diff --git a/lib/stdlib/test/supervisor_SUITE.erl b/lib/stdlib/test/supervisor_SUITE.erl index d9f2e82b31..569c66959e 100644 --- a/lib/stdlib/test/supervisor_SUITE.erl +++ b/lib/stdlib/test/supervisor_SUITE.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 1996-2011. All Rights Reserved. +%% Copyright Ericsson AB 1996-2012. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -671,17 +671,24 @@ faulty_application_shutdown(Config) when is_list(Config) -> %% Start faulty app code:add_patha(EbinDir), - % {error, - % {{shutdown, - % {undef,[ - % {an_undefined_module_with,an_undefined_function,[argument1,argument2],[]}, - % {app_faulty_server,init,1,[{file,"app_faulty/src/app_faulty_server.erl"},{line,16}]}, - % {gen_server,init_it,6,[{file,"gen_server.erl"},{line,304}]}, - % {proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,227}]} - % ]}},{app_faulty,start,[normal,[]]}}} + %% {error, + %% {{shutdown, + %% {failed_to_start_child, + %% app_faulty, + %% {undef, + %% [{an_undefined_module_with,an_undefined_function,[argument1,argument2], + %% []}, + %% {app_faulty_server,init,1, + %% [{file,"app_faulty/src/app_faulty_server.erl"},{line,16}]}, + %% {gen_server,init_it,6, + %% [{file,"gen_server.erl"},{line,304}]}, + %% {proc_lib,init_p_do_apply,3, + %% [{file,"proc_lib.erl"},{line,227}]}]}}}, + %% {app_faulty,start,[normal,[]]}}} {error, Error} = application:start(app_faulty), - {{shutdown, {undef, CallStack}},{app_faulty,start,_}} = Error, + {{shutdown, {failed_to_start_child,app_faulty,{undef, CallStack}}}, + {app_faulty,start,_}} = Error, [{an_undefined_module_with,an_undefined_function,_,_}|_] = CallStack, ok = application:unload(app_faulty), ok. -- cgit v1.2.3