From 30cdfe871577f9622297fb5ed792b117894ef1f2 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Wed, 27 Jul 2016 14:41:07 +0200 Subject: Rewrite gen_statem for M:callback_mode/0 --- lib/stdlib/src/gen_statem.erl | 133 +++++++++++++++++++++--------------------- 1 file changed, 68 insertions(+), 65 deletions(-) diff --git a/lib/stdlib/src/gen_statem.erl b/lib/stdlib/src/gen_statem.erl index c02e6a1a19..6d7b461ee3 100644 --- a/lib/stdlib/src/gen_statem.erl +++ b/lib/stdlib/src/gen_statem.erl @@ -24,7 +24,7 @@ [start/3,start/4,start_link/3,start_link/4, stop/1,stop/3, cast/2,call/2,call/3, - enter_loop/5,enter_loop/6,enter_loop/7, + enter_loop/4,enter_loop/5,enter_loop/6, reply/1,reply/2]). %% gen callbacks @@ -63,8 +63,8 @@ {To :: pid(), Tag :: term()}. % Reply-to specifier for call -type state() :: - state_name() | % For state callback function StateName/5 - term(). % For state callback function handle_event/5 + state_name() | % For StateName/3 callback functios + term(). % For handle_event/4 callback function -type state_name() :: atom(). @@ -174,28 +174,33 @@ %% an {ok, ...} tuple. Thereafter the state callbacks are called %% for all events to this server. -callback init(Args :: term()) -> - {callback_mode(), state(), data()} | - {callback_mode(), state(), data(), [action()] | action()} | + {ok, state(), data()} | + {ok, state(), data(), [action()] | action()} | 'ignore' | {'stop', Reason :: term()}. -%% Example state callback for callback_mode() =:= state_functions -%% state name 'state_name'. +%% This callback shall return the callback mode of the callback module. %% -%% In this mode all states has to be type state_name() i.e atom(). +%% It is called once after init/0 and code_change/4 but before +%% the first state callback StateName/3 or handle_event/4. +-callback callback_mode() -> callback_mode(). + +%% Example state callback for StateName = 'state_name' +%% when callback_mode() =:= state_functions. +%% +%% In this mode all states has to be of type state_name() i.e atom(). %% -%% Note that state callbacks and only state callbacks have arity 5 -%% and that is intended. +%% Note that the only callbacks that have arity 3 are these +%% StateName/3 callbacks and terminate/3, so the state name +%% 'terminate' is unusable in this mode. -callback state_name( event_type(), EventContent :: term(), Data :: data()) -> state_function_result(). %% -%% State callback for callback_mode() =:= handle_event_function. -%% -%% Note that state callbacks and only state callbacks have arity 5 -%% and that is intended. +%% State callback for all states +%% when callback_mode() =:= handle_event_function. -callback handle_event( event_type(), EventContent :: term(), @@ -219,9 +224,7 @@ OldState :: state(), OldData :: data(), Extra :: term()) -> - {CallbackMode :: callback_mode(), - NewState :: state(), - NewData :: data()} | + {ok, NewState :: state(), NewData :: data()} | (Reason :: term()). %% Format the callback module state in some sensible that is @@ -240,10 +243,13 @@ [init/1, % One may use enter_loop/5,6,7 instead format_status/2, % Has got a default implementation %% - state_name/3, % Example for callback_mode =:= state_functions: - %% there has to be a StateName/5 callback function for every StateName. + state_name/3, % Example for callback_mode() =:= state_functions: + %% there has to be a StateName/3 callback function + %% for every StateName in your state machine but the state name + %% 'state_name' does of course not have to be used. %% - handle_event/4]). % For callback_mode =:= handle_event_function + handle_event/4 % For callback_mode() =:= handle_event_function + ]). %% Type validation functions callback_mode(CallbackMode) -> @@ -451,43 +457,35 @@ reply({To,Tag}, Reply) when is_pid(To) -> %% the same arguments as you would have returned from init/1 -spec enter_loop( Module :: module(), Opts :: [debug_opt()], - CallbackMode :: callback_mode(), State :: state(), Data :: data()) -> no_return(). -enter_loop(Module, Opts, CallbackMode, State, Data) -> - enter_loop(Module, Opts, CallbackMode, State, Data, self()). +enter_loop(Module, Opts, State, Data) -> + enter_loop(Module, Opts, State, Data, self()). %% -spec enter_loop( Module :: module(), Opts :: [debug_opt()], - CallbackMode :: callback_mode(), State :: state(), Data :: data(), Server_or_Actions :: server_name() | pid() | [action()]) -> no_return(). -enter_loop(Module, Opts, CallbackMode, State, Data, Server_or_Actions) -> +enter_loop(Module, Opts, State, Data, Server_or_Actions) -> if is_list(Server_or_Actions) -> - enter_loop( - Module, Opts, CallbackMode, State, Data, - self(), Server_or_Actions); + enter_loop(Module, Opts, State, Data, self(), Server_or_Actions); true -> - enter_loop( - Module, Opts, CallbackMode, State, Data, - Server_or_Actions, []) + enter_loop(Module, Opts, State, Data, Server_or_Actions, []) end. %% -spec enter_loop( Module :: module(), Opts :: [debug_opt()], - CallbackMode :: callback_mode(), State :: state(), Data :: data(), Server :: server_name() | pid(), Actions :: [action()] | action()) -> no_return(). -enter_loop(Module, Opts, CallbackMode, State, Data, Server, Actions) -> +enter_loop(Module, Opts, State, Data, Server, Actions) -> is_atom(Module) orelse error({atom,Module}), - callback_mode(CallbackMode) orelse error({callback_mode,CallbackMode}), Parent = gen:get_parent(), - enter(Module, Opts, CallbackMode, State, Data, Server, Actions, Parent). + enter(Module, Opts, State, Data, Server, Actions, Parent). %%--------------------------------------------------------------------------- %% API helpers @@ -515,7 +513,7 @@ send(Proc, Msg) -> end. %% Here the init_it/6 and enter_loop/5,6,7 functions converge -enter(Module, Opts, CallbackMode, State, Data, Server, Actions, Parent) -> +enter(Module, Opts, State, Data, Server, Actions, Parent) -> %% The values should already have been type checked Name = gen:get_proc_name(Server), Debug = gen:debug_options(Name, Opts), @@ -531,7 +529,7 @@ enter(Module, Opts, CallbackMode, State, Data, Server, Actions, Parent) -> [Actions,{postpone,false}] end, S = #{ - callback_mode => CallbackMode, + callback_mode => undefined, module => Module, name => Name, %% All fields below will be replaced according to the arguments to @@ -569,30 +567,12 @@ init_it(Starter, Parent, ServerRef, Module, Args, Opts) -> init_result(Starter, Parent, ServerRef, Module, Result, Opts) -> case Result of - {CallbackMode,State,Data} -> - case callback_mode(CallbackMode) of - true -> - proc_lib:init_ack(Starter, {ok,self()}), - enter( - Module, Opts, CallbackMode, State, Data, - ServerRef, [], Parent); - false -> - Error = {callback_mode,CallbackMode}, - proc_lib:init_ack(Starter, {error,Error}), - exit(Error) - end; - {CallbackMode,State,Data,Actions} -> - case callback_mode(CallbackMode) of - true -> - proc_lib:init_ack(Starter, {ok,self()}), - enter( - Module, Opts, CallbackMode, State, Data, - ServerRef, Actions, Parent); - false -> - Error = {callback_mode,CallbackMode}, - proc_lib:init_ack(Starter, {error,Error}), - exit(Error) - end; + {ok,State,Data} -> + proc_lib:init_ack(Starter, {ok,self()}), + enter(Module, Opts, State, Data, ServerRef, [], Parent); + {ok,State,Data,Actions} -> + proc_lib:init_ack(Starter, {ok,self()}), + enter(Module, Opts, State, Data, ServerRef, Actions, Parent); {stop,Reason} -> gen:unregister_name(ServerRef), proc_lib:init_ack(Starter, {error,Reason}), @@ -631,11 +611,9 @@ system_code_change( Result -> Result end of - {CallbackMode,NewState,NewData} -> - callback_mode(CallbackMode) orelse - error({callback_mode,CallbackMode}), + {ok,NewState,NewData} -> {ok, - S#{callback_mode := CallbackMode, + S#{callback_mode := undefined, state := NewState, data := NewData}}; {ok,_} = Error -> @@ -857,6 +835,31 @@ loop_events_done(Parent, Debug, S, Timer, State, Data, P, Hibernate) -> timer := Timer}, loop(Parent, Debug, NewS). +loop_event( + Parent, Debug, + #{callback_mode := undefined, + module := Module} = S, + Events, + State, Data, P, Event, Hibernate) -> + %% Cache the callback_mode() value + case + try Module:callback_mode() + catch + Result -> Result + end + of + CallbackMode -> + case callback_mode(CallbackMode) of + true -> + loop_event( + Parent, Debug, + S#{callback_mode := CallbackMode}, + Events, + State, Data, P, Event, Hibernate); + false -> + error({callback_mode,CallbackMode}) + end + end; loop_event( Parent, Debug, #{callback_mode := CallbackMode, -- cgit v1.2.3 From c4f38aca1cd915797dcdba62d790e4b9f0bdbd12 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Wed, 27 Jul 2016 14:41:27 +0200 Subject: Rewrite gen_statem TCs for M:callback_mode/0 --- lib/stdlib/test/gen_statem_SUITE.erl | 113 ++++++++++++++++++----------------- 1 file changed, 59 insertions(+), 54 deletions(-) diff --git a/lib/stdlib/test/gen_statem_SUITE.erl b/lib/stdlib/test/gen_statem_SUITE.erl index 364314f91b..d90119de54 100644 --- a/lib/stdlib/test/gen_statem_SUITE.erl +++ b/lib/stdlib/test/gen_statem_SUITE.erl @@ -37,33 +37,32 @@ all() -> {group, stop_handle_event}, {group, abnormal}, {group, abnormal_handle_event}, - shutdown, stop_and_reply, event_order, + shutdown, stop_and_reply, event_order, code_change, {group, sys}, hibernate, enter_loop]. groups() -> - [{start, [], - [start1, start2, start3, start4, start5, start6, start7, - start8, start9, start10, start11, start12, next_events]}, - {start_handle_event, [], - [start1, start2, start3, start4, start5, start6, start7, - start8, start9, start10, start11, start12, next_events]}, - {stop, [], - [stop1, stop2, stop3, stop4, stop5, stop6, stop7, stop8, stop9, stop10]}, - {stop_handle_event, [], - [stop1, stop2, stop3, stop4, stop5, stop6, stop7, stop8, stop9, stop10]}, - {abnormal, [], [abnormal1, abnormal2]}, - {abnormal_handle_event, [], [abnormal1, abnormal2]}, - {sys, [], - [sys1, code_change, - call_format_status, - error_format_status, terminate_crash_format, - get_state, replace_state]}, - {sys_handle_event, [], - [sys1, - call_format_status, - error_format_status, terminate_crash_format, - get_state, replace_state]}]. + [{start, [], tcs(start)}, + {start_handle_event, [], tcs(start)}, + {stop, [], tcs(stop)}, + {stop_handle_event, [], tcs(stop)}, + {abnormal, [], tcs(abnormal)}, + {abnormal_handle_event, [], tcs(abnormal)}, + {sys, [], tcs(sys)}, + {sys_handle_event, [], tcs(sys)}]. + +tcs(start) -> + [start1, start2, start3, start4, start5, start6, start7, + start8, start9, start10, start11, start12, next_events]; +tcs(stop) -> + [stop1, stop2, stop3, stop4, stop5, stop6, stop7, stop8, stop9, stop10]; +tcs(abnormal) -> + [abnormal1, abnormal2]; +tcs(sys) -> + [sys1, call_format_status, + error_format_status, terminate_crash_format, + get_state, replace_state]. + init_per_suite(Config) -> Config. @@ -633,11 +632,13 @@ sys1(Config) -> sys:resume(Pid), stop_it(Pid). -code_change(Config) -> - Mode = handle_event_function, - {ok,Pid} = gen_statem:start(?MODULE, start_arg(Config, []), []), +code_change(_Config) -> + {ok,Pid} = + gen_statem:start( + ?MODULE, {callback_mode,state_functions,[]}, []), {idle,data} = sys:get_state(Pid), sys:suspend(Pid), + Mode = handle_event_function, sys:change_code(Pid, ?MODULE, old_vsn, Mode), sys:resume(Pid), {idle,{old_vsn,data,Mode}} = sys:get_state(Pid), @@ -1029,11 +1030,7 @@ enter_loop(_Config) -> end, %% Process not started using proc_lib - CallbackMode = state_functions, - Pid4 = - spawn_link( - gen_statem, enter_loop, - [?MODULE,[],CallbackMode,state0,[]]), + Pid4 = spawn_link(gen_statem, enter_loop, [?MODULE,[],state0,[]]), receive {'EXIT',Pid4,process_was_not_started_by_proc_lib} -> ok @@ -1107,21 +1104,18 @@ enter_loop(Reg1, Reg2) -> anon -> ignore end, proc_lib:init_ack({ok, self()}), - CallbackMode = state_functions, case Reg2 of local -> gen_statem:enter_loop( - ?MODULE, [], CallbackMode, state0, [], {local,armitage}); + ?MODULE, [], state0, [], {local,armitage}); global -> gen_statem:enter_loop( - ?MODULE, [], CallbackMode, state0, [], {global,armitage}); + ?MODULE, [], state0, [], {global,armitage}); via -> gen_statem:enter_loop( - ?MODULE, [], CallbackMode, state0, [], - {via, dummy_via, armitage}); + ?MODULE, [], state0, [], {via, dummy_via, armitage}); anon -> - gen_statem:enter_loop( - ?MODULE, [], CallbackMode, state0, []) + gen_statem:enter_loop(?MODULE, [], state0, []) end. @@ -1266,33 +1260,39 @@ init(stop_shutdown) -> {stop,shutdown}; init(sleep) -> ?t:sleep(1000), - {state_functions,idle,data}; + {ok,idle,data}; init(hiber) -> - {state_functions,hiber_idle,[]}; + {ok,hiber_idle,[]}; init(hiber_now) -> - {state_functions,hiber_idle,[],[hibernate]}; + {ok,hiber_idle,[],[hibernate]}; init({data, Data}) -> - {state_functions,idle,Data}; + {ok,idle,Data}; init({callback_mode,CallbackMode,Arg}) -> - case init(Arg) of - {_,State,Data,Ops} -> - {CallbackMode,State,Data,Ops}; - {_,State,Data} -> - {CallbackMode,State,Data}; - Other -> - Other - end; + ets:new(?MODULE, [named_table,private]), + ets:insert(?MODULE, {callback_mode,CallbackMode}), + init(Arg); init({map_statem,#{init := Init}=Machine}) -> + ets:new(?MODULE, [named_table,private]), + ets:insert(?MODULE, {callback_mode,handle_event_function}), case Init() of {ok,State,Data,Ops} -> - {handle_event_function,State,[Data|Machine],Ops}; + {ok,State,[Data|Machine],Ops}; {ok,State,Data} -> - {handle_event_function,State,[Data|Machine]}; + {ok,State,[Data|Machine]}; Other -> Other end; init([]) -> - {state_functions,idle,data}. + {ok,idle,data}. + +callback_mode() -> + try ets:lookup(?MODULE, callback_mode) of + [{callback_mode,CallbackMode}] -> + CallbackMode + catch + error:badarg -> + state_functions + end. terminate(_, _State, crash_terminate) -> exit({crash,terminate}); @@ -1568,7 +1568,12 @@ wrap_result(Result) -> code_change(OldVsn, State, Data, CallbackMode) -> - {CallbackMode,State,{OldVsn,Data,CallbackMode}}. + io:format( + "code_change(~p, ~p, ~p, ~p)~n", [OldVsn,State,Data,CallbackMode]), + ets:insert(?MODULE, {callback_mode,CallbackMode}), + io:format( + "code_change(~p, ~p, ~p, ~p)~n", [OldVsn,State,Data,CallbackMode]), + {ok,State,{OldVsn,Data,CallbackMode}}. format_status(terminate, [_Pdict,State,Data]) -> {formatted,State,Data}; -- cgit v1.2.3 From 259c8c7bde51f0b25707f0101195aff22650e952 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Wed, 27 Jul 2016 14:41:45 +0200 Subject: Rewrite gen_statem docs for M:callback_mode/0 --- lib/stdlib/doc/src/gen_statem.xml | 203 +++++++++++++++++++------------- system/doc/design_principles/statem.xml | 79 +++++++------ 2 files changed, 166 insertions(+), 116 deletions(-) diff --git a/lib/stdlib/doc/src/gen_statem.xml b/lib/stdlib/doc/src/gen_statem.xml index ed44eef912..f18c1087ab 100644 --- a/lib/stdlib/doc/src/gen_statem.xml +++ b/lib/stdlib/doc/src/gen_statem.xml @@ -97,6 +97,9 @@ gen_statem module Callback module gen_statem:start gen_statem:start_link -----> Module:init/1 +Server start or code change + -----> Module:callback_mode/0 + gen_statem:stop -----> Module:terminate/3 gen_statem:call @@ -127,7 +130,8 @@ erlang:'!' -----> Module:StateName/3 in a gen_statem is the callback function that is called for all events in this state. It is selected depending on which callback mode - that the implementation specifies when the server starts. + that the callback module defines with the callback function + Module:callback_mode/0.

When the @@ -138,9 +142,9 @@ erlang:'!' -----> Module:StateName/3 This gathers all code for a specific state in one function as the gen_statem engine branches depending on state name. - Notice that in this mode the mandatory callback function + Notice the fact that there is a mandatory callback function Module:terminate/3 - makes the state name terminate unusable. + makes the state name terminate unusable in this mode.

When the @@ -249,11 +253,10 @@ erlang:'!' -----> Module:StateName/3 -behaviour(gen_statem). -export([start/0,push/0,get_count/0,stop/0]). --export([terminate/3,code_change/4,init/1]). +-export([terminate/3,code_change/4,init/1,callback_mode/0]). -export([on/3,off/3]). name() -> pushbutton_statem. % The registered server name -callback_mode() -> state_functions. %% API. This example uses a registered name name() %% and does not link to the caller. @@ -270,15 +273,14 @@ stop() -> terminate(_Reason, _State, _Data) -> void. code_change(_Vsn, State, Data, _Extra) -> - {callback_mode(),State,Data}. + {ok,State,Data}. init([]) -> - %% Set the callback mode and initial state + data. - %% Data is used only as a counter. + %% Set the initial state + data. Data is used only as a counter. State = off, Data = 0, - {callback_mode(),State,Data}. - + {ok,State,Data}. +callback_mode() -> state_functions. -%%% State functions +%%% State function(s) off({call,From}, push, Data) -> %% Go to 'on', increment count and reply @@ -326,18 +328,13 @@ ok To compare styles, here follows the same example using callback mode state_functions, or rather the code to replace - from function init/1 of the pushbutton.erl + after function init/1 of the pushbutton.erl example file above:

-init([]) -> - %% Set the callback mode and initial state + data. - %% Data is used only as a counter. - State = off, Data = 0, - {handle_event_function,State,Data}. - +callback_mode() -> handle_event_function. -%%% Event handling +%%% State function(s) handle_event({call,From}, push, off, Data) -> %% Go to 'on', increment count and reply @@ -426,8 +423,8 @@ handle_event(_, _, State, Data) ->

Debug option that can be used when starting - a gen_statem server through, for example, - enter_loop/5. + a gen_statem server through, + enter_loop/4-6.

For every entry in Dbgs, @@ -525,12 +522,9 @@ handle_event(_, _, State, Data) ->

The callback mode is selected when starting the - gen_statem using the return value from - Module:init/1 - or when calling - enter_loop/5,6,7, - and with the return value from - Module:code_change/4. + gen_statem and after code change + using the return value from + Module:callback_mode/0.

state_functions @@ -691,7 +685,7 @@ handle_event(_, _, State, Data) -> state function, from Module:init/1 or by giving them to - enter_loop/6,7. + enter_loop/5,6.

Actions are executed in the containing list order. @@ -958,35 +952,36 @@ handle_event(_, _, State, Data) -> - + Enter the gen_statem receive loop.

The same as - enter_loop/7 - except that no + enter_loop/6 + with Actions = [] except that no server_name() - must have been registered. + must have been registered. This creates an anonymous server.

- + Enter the gen_statem receive loop.

If Server_or_Actions is a list(), the same as - enter_loop/7 + enter_loop/6 except that no server_name() must have been registered and Actions = Server_or_Actions. + This creates an anonymous server.

Otherwise the same as - enter_loop/7 + enter_loop/6 with Server = Server_or_Actions and Actions = []. @@ -995,7 +990,7 @@ handle_event(_, _, State, Data) -> - + Enter the gen_statem receive loop.

@@ -1015,21 +1010,31 @@ handle_event(_, _, State, Data) -> the gen_statem behavior provides.

- Module, Opts, and - Server have the same meanings - as when calling + Module, Opts + have the same meaning as when calling start[_link]/3,4. +

+

+ If Server is self() an anonymous + server is created just as when using + start[_link]/3. + If Server is a + server_name() + a named server is created just as when using + start[_link]/4. However, the server_name() name must have been registered accordingly - before this function is called.

+ before this function is called. +

- CallbackMode, State, - Data, and Actions + State, Data, + and Actions have the same meanings as in the return value of Module:init/1. - Also, the callback module Module - does not need to export an init/1 function. + Also, the callback module does not need to export a + Module:init/1 + function.

The function fails if the calling process was not started by a @@ -1252,6 +1257,54 @@ handle_event(_, _, State, Data) -> + + Module:callback_mode() -> CallbackMode + Update the internal state during upgrade/downgrade. + + + CallbackMode = + callback_mode() + + + +

+ This function is called by a gen_statem + when it needs to find out the + callback mode + of the callback module. The value is cached by gen_statem + for efficiency reasons, so this function is only called + once after server start and after code change, + but before the first + state function + is called. More occasions may be added in future versions + of gen_statem. +

+

+ Server start happens either when + Module:init/1 + returns or when + enter_loop/4-6 + is called. Code change happens when + Module:code_change/4 + returns. +

+

+ This function can use + erlang:throw/1 + to return CallbackMode, just for symmetry reasons. + There should be no actual reason to use this. +

+ +

+ If this function's body does not consist of solely one of two + possible + atoms + the callback module is doing something strange. +

+
+
+
+ Module:code_change(OldVsn, OldState, OldData, Extra) -> Result @@ -1262,11 +1315,7 @@ handle_event(_, _, State, Data) ->   Vsn = term() OldState = NewState = term() Extra = term() - Result = {CallbackMode,NewState,NewData} | Reason - - CallbackMode = - callback_mode() - + Result = {ok,NewState,NewData} | Reason OldState = NewState = state() @@ -1295,21 +1344,6 @@ handle_event(_, _, State, Data) -> Module. If no such attribute is defined, the version is the checksum of the Beam file.

- -

- If you would dare to change - callback mode - during release upgrade/downgrade, the upgrade is no problem, - as the new code surely knows what callback mode - it needs. However, for a downgrade this function must - know from argument Extra that comes from the - sasl:appup - file what callback mode the old code did use. - It can also be possible to figure this out - from argument {down,Vsn}, as Vsn - in effect defines the old callback module version. -

-

OldState and OldData is the internal state of the gen_statem. @@ -1321,16 +1355,16 @@ handle_event(_, _, State, Data) ->

If successful, the function must return the updated internal state in an - {CallbackMode,NewState,NewData} tuple. + {ok,NewState,NewData} tuple.

If the function returns a failure Reason, the ongoing upgrade fails and rolls back to the old release. - Note that Reason can not be a 3-tuple since that - will be regarded as a - {CallbackMode,NewState,NewData} tuple, + Note that Reason can not be an {ok,_,_} tuple + since that will be regarded as a + {ok,NewState,NewData} tuple, and that a tuple matching {ok,_} - is an invalid failure Reason. + is an also invalid failure Reason. It is recommended to use an atom as Reason since it will be wrapped in an {error,Reason} tuple.

@@ -1344,16 +1378,14 @@ handle_event(_, _, State, Data) -> Module:init(Args) -> Result - Initialize process and internal state. + + Optional function for initializing process and internal state. + Args = term() - Result = {CallbackMode,State,Data} -  | {CallbackMode,State,Data,Actions} + Result = {ok,State,Data} +  | {ok,State,Data,Actions}  | {stop,Reason} | ignore - - CallbackMode = - callback_mode() - State = state() Data = data() @@ -1372,7 +1404,7 @@ handle_event(_, _, State, Data) -> start_link/3,4 or start/3,4, - this function is called by the new process to initialize + this optional function is called by the new process to initialize the implementation state and server data.

@@ -1381,11 +1413,8 @@ handle_event(_, _, State, Data) ->

If the initialization is successful, the function is to - return {CallbackMode,State,Data} or - {CallbackMode,State,Data,Actions}. - CallbackMode selects the - callback mode - of the gen_statem. + return {ok,State,Data} or + {ok,State,Data,Actions}. State is the initial state() and Data the initial server @@ -1408,6 +1437,16 @@ handle_event(_, _, State, Data) -> erlang:throw/1 to return Result.

+ +

+ This callback is optional, so a callback module does not need + to export it, but most do. If this function is not exported, + the gen_statem should be started through + proc_lib + and + enter_loop/4-6. +

+
diff --git a/system/doc/design_principles/statem.xml b/system/doc/design_principles/statem.xml index aea623851a..1351997bc1 100644 --- a/system/doc/design_principles/statem.xml +++ b/system/doc/design_principles/statem.xml @@ -52,7 +52,7 @@
Event-Driven State Machines

- Established Automata theory does not deal much with + Established Automata Theory does not deal much with how a state transition is triggered, but assumes that the output is a function of the input (and the state) and that they are @@ -226,11 +226,10 @@ handle_event(EventType, EventContent, State, Data) -> -module(code_lock). -behaviour(gen_statem). -define(NAME, code_lock). --define(CALLBACK_MODE, state_functions). -export([start_link/1]). -export([button/1]). --export([init/1,terminate/3,code_change/4]). +-export([init/1,callback_mode/0,terminate/3,code_change/4]). -export([locked/3,open/3]). start_link(Code) -> @@ -242,7 +241,10 @@ button(Digit) -> init(Code) -> do_lock(), Data = #{code => Code, remaining => Code}, - {?CALLBACK_MODE,locked,Data}. + {ok,locked,Data}. + +callback_mode() -> + state_functions. locked( cast, {button,Digit}, @@ -273,7 +275,7 @@ terminate(_Reason, State, _Data) -> State =/= locked andalso do_lock(), ok. code_change(_Vsn, State, Data, _Extra) -> - {?CALLBACK_MODE,State,Data}. + {ok,State,Data}. ]]>

The code is explained in the next sections.

@@ -343,14 +345,8 @@ start_link(Code) ->

If name registration succeeds, the new gen_statem process calls callback function code_lock:init(Code). - This function is expected to return {CallbackMode,State,Data}, - where - CallbackMode - selects callback module state function mode, in this case - state_functions - through macro ?CALLBACK_MODE. That is, each state - has got its own handler function. - State is the initial state of the gen_statem, + This function is expected to return {ok,State,Data}, + where State is the initial state of the gen_statem, in this case locked; assuming that the door is locked to begin with. Data is the internal server data of the gen_statem. Here the server data is a map @@ -359,11 +355,12 @@ start_link(Code) -> that stores the remaining correct button sequence (the same as the code to begin with).

+ do_lock(), Data = #{code => Code, remaining => Code}, - {?CALLBACK_MODE,locked,Data}. + {ok,locked,Data}. ]]>

Function gen_statem:start_link @@ -380,6 +377,21 @@ init(Code) -> can be used to start a standalone gen_statem, that is, a gen_statem that is not part of a supervision tree.

+ + + state_functions. + ]]> +

+ Function + Module:callback_mode/0 + selects the + CallbackMode + for the callback module, in this case + state_functions. + That is, each state has got its own handler function. +

+ @@ -532,13 +544,12 @@ handle_event({call,From}, code_length, #{code := Code} = Data) -> and then depending on state:

+ handle_event_function. handle_event(cast, {button,Digit}, State, #{code := Code} = Data) -> case State of @@ -962,16 +973,13 @@ do_unlock() ->

process_flag(trap_exit, true), Data = #{code => Code}, - enter(?CALLBACK_MODE, locked, Data). + enter(ok, locked, Data). -... +callback_mode() -> + state_functions. locked(internal, enter, _Data) -> do_lock(), @@ -1027,11 +1035,10 @@ enter(Tag, State, Data) -> -module(code_lock). -behaviour(gen_statem). -define(NAME, code_lock_2). --define(CALLBACK_MODE, state_functions). -export([start_link/1,stop/0]). -export([button/1,code_length/0]). --export([init/1,terminate/3,code_change/4]). +-export([init/1,callback_mode/0,terminate/3,code_change/4]). -export([locked/3,open/3]). start_link(Code) -> @@ -1047,7 +1054,10 @@ code_length() -> init(Code) -> process_flag(trap_exit, true), Data = #{code => Code}, - enter(?CALLBACK_MODE, locked, Data). + enter(ok, locked, Data). + +callback_mode() -> + state_functions. locked(internal, enter, #{code := Code} = Data) -> do_lock(), @@ -1091,7 +1101,7 @@ terminate(_Reason, State, _Data) -> State =/= locked andalso do_lock(), ok. code_change(_Vsn, State, Data, _Extra) -> - {?CALLBACK_MODE,State,Data}. + {ok,State,Data}. ]]> @@ -1105,13 +1115,12 @@ code_change(_Vsn, State, Data, _Extra) -> entry actions, so this example first branches depending on state:

+ handle_event_function. %% State: locked handle_event(internal, enter, locked, #{code := Code} = Data) -> @@ -1273,11 +1282,10 @@ format_status(Opt, [_PDict,State,Data]) -> -module(code_lock). -behaviour(gen_statem). -define(NAME, code_lock_3). --define(CALLBACK_MODE, handle_event_function). -export([start_link/2,stop/0]). -export([button/1,code_length/0,set_lock_button/1]). --export([init/1,terminate/3,code_change/4,format_status/2]). +-export([init/1,callback_mode/0,terminate/3,code_change/4,format_status/2]). -export([handle_event/4]). start_link(Code, LockButton) -> @@ -1296,7 +1304,10 @@ set_lock_button(LockButton) -> init({Code,LockButton}) -> process_flag(trap_exit, true), Data = #{code => Code, remaining => undefined, timer => undefined}, - enter(?CALLBACK_MODE, {locked,LockButton}, Data, []). + enter(ok, {locked,LockButton}, Data, []). + +callback_mode() -> + handle_event_function. handle_event( {call,From}, {set_lock_button,NewLockButton}, @@ -1366,7 +1377,7 @@ terminate(_Reason, State, _Data) -> State =/= locked andalso do_lock(), ok. code_change(_Vsn, State, Data, _Extra) -> - {?CALLBACK_MODE,State,Data}. + {ok,State,Data}. format_status(Opt, [_PDict,State,Data]) -> StateData = {State, -- cgit v1.2.3 From 46a93b31a9b94ffb478393788376f78249e60a1f Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Mon, 25 Jul 2016 15:19:44 +0200 Subject: Rewrite Tools for gen_statem M:callback_mode/0 --- lib/tools/doc/src/erlang_mode.xml | 9 ++- lib/tools/emacs/erlang-skels.el | 145 ++++++++++++++++++++++++++++++++------ 2 files changed, 131 insertions(+), 23 deletions(-) diff --git a/lib/tools/doc/src/erlang_mode.xml b/lib/tools/doc/src/erlang_mode.xml index 00cf5196b4..7fef74813b 100644 --- a/lib/tools/doc/src/erlang_mode.xml +++ b/lib/tools/doc/src/erlang_mode.xml @@ -252,7 +252,14 @@ behavior gen_event - skeleton for the OTP gen_event behavior gen_fsm - skeleton for the OTP gen_fsm behavior - gen_statem - skeleton for the OTP gen_statem behavior + + gen_statem (StateName/3) - skeleton for the OTP gen_statem behavior + using state name functions + + + gen_statem (handle_event/4) - skeleton for the OTP gen_statem behavior + using one state function + Library module - skeleton for a module that does not implement a process. Corba callback - skeleton for a Corba callback module. diff --git a/lib/tools/emacs/erlang-skels.el b/lib/tools/emacs/erlang-skels.el index c1152f31a4..43763d2c8a 100644 --- a/lib/tools/emacs/erlang-skels.el +++ b/lib/tools/emacs/erlang-skels.el @@ -56,8 +56,10 @@ erlang-skel-gen-event erlang-skel-header) ("gen_fsm" "gen-fsm" erlang-skel-gen-fsm erlang-skel-header) - ("gen_statem" "gen-statem" - erlang-skel-gen-statem erlang-skel-header) + ("gen_statem (StateName/3)" "gen-statem-StateName" + erlang-skel-gen-statem-StateName erlang-skel-header) + ("gen_statem (handle_event/4)" "gen-statem-handle-event" + erlang-skel-gen-statem-handle-event erlang-skel-header) ("wx_object" "wx-object" erlang-skel-wx-object erlang-skel-header) ("Library module" "gen-lib" @@ -860,7 +862,7 @@ Please see the function `tempo-define-template'.") "*The template of a gen_fsm. Please see the function `tempo-define-template'.") -(defvar erlang-skel-gen-statem +(defvar erlang-skel-gen-statem-StateName '((erlang-skel-include erlang-skel-large-header) "-behaviour(gen_statem)." n n @@ -868,9 +870,8 @@ Please see the function `tempo-define-template'.") "-export([start_link/0])." n n "%% gen_statem callbacks" n - "-export([init/1, terminate/3, code_change/4])." n + "-export([callback_mode/0, init/1, terminate/3, code_change/4])." n "-export([state_name/3])." n - "-export([handle_event/4])." n n "-define(SERVER, ?MODULE)." n n @@ -899,30 +900,37 @@ Please see the function `tempo-define-template'.") (erlang-skel-separator-start 2) "%% @private" n "%% @doc" n + "%% Define the callback_mode() for this callback module." n + (erlang-skel-separator-end 2) + "-spec callback_mode() -> gen_statem:callback_mode()." n + "callback_mode() -> state_functions." n + n + (erlang-skel-separator-start 2) + "%% @private" n + "%% @doc" n "%% Whenever a gen_statem is started using gen_statem:start/[3,4] or" n "%% gen_statem:start_link/[3,4], this function is called by the new" n "%% process to initialize." n (erlang-skel-separator-end 2) "-spec init(Args :: term()) ->" n> - "{gen_statem:callback_mode()," n> - "State :: term(), Data :: term()} |" n> - "{gen_statem:callback_mode()," n> - "State :: term(), Data :: term()," n> + "{ok, State :: term(), Data :: term()} |" n> + "{ok, State :: term(), Data :: term()," n> "[gen_statem:action()] | gen_statem:action()} |" n> "ignore |" n> "{stop, Reason :: term()}." n "init([]) ->" n> - "{state_functions, state_name, #data{}}." n + "{ok, state_name, #data{}}." n n (erlang-skel-separator-start 2) "%% @private" n "%% @doc" n - "%% If the gen_statem runs with CallbackMode =:= state_functions" n - "%% there should be one instance of this function for each possible" n - "%% state name. Whenever a gen_statem receives an event," n - "%% the instance of this function with the same name" n - "%% as the current state name StateName is called to" n - "%% handle the event." n + "%% There should be one function like this for each state name." n + "%% Whenever a gen_statem receives an event, the function " n + "%% with the name of the current state (StateName) " n + "%% is called to handle the event." n + "%%" n + "%% NOTE: If there is an exported function handle_event/4, it is called" n + "%% instead of StateName/3 functions like this!" n (erlang-skel-separator-end 2) "-spec state_name(" n> "gen_statem:event_type(), Msg :: term()," n> @@ -934,8 +942,102 @@ Please see the function `tempo-define-template'.") (erlang-skel-separator-start 2) "%% @private" n "%% @doc" n - "%% If the gen_statem runs with CallbackMode =:= handle_event_function" n - "%% this function is called for every event a gen_statem receives." n + "%% This function is called by a gen_statem when it is about to" n + "%% terminate. It should be the opposite of Module:init/1 and do any" n + "%% necessary cleaning up. When it returns, the gen_statem terminates with" n + "%% Reason. The return value is ignored." n + (erlang-skel-separator-end 2) + "-spec terminate(Reason :: term(), State :: term(), Data :: term()) ->" n> + "any()." n + "terminate(_Reason, _State, _Data) ->" n> + "void." n + n + (erlang-skel-separator-start 2) + "%% @private" n + "%% @doc" n + "%% Convert process state when code is changed" n + (erlang-skel-separator-end 2) + "-spec code_change(" n> + "OldVsn :: term() | {down,term()}," n> + "State :: term(), Data :: term(), Extra :: term()) ->" n> + "{ok, NewState :: term(), NewData :: term()} |" n> + "(Reason :: term())." n + "code_change(_OldVsn, State, Data, _Extra) ->" n> + "{ok, State, Data}." n + n + (erlang-skel-double-separator-start 3) + "%%% Internal functions" n + (erlang-skel-double-separator-end 3) + ) + "*The template of a gen_statem (StateName/3). +Please see the function `tempo-define-template'.") + +(defvar erlang-skel-gen-statem-handle-event + '((erlang-skel-include erlang-skel-large-header) + "-behaviour(gen_statem)." n n + + "%% API" n + "-export([start_link/0])." n + n + "%% gen_statem callbacks" n + "-export([callback_mode/0, init/1, terminate/3, code_change/4])." n + "-export([handle_event/4])." n + n + "-define(SERVER, ?MODULE)." n + n + "-record(data, {})." n + n + (erlang-skel-double-separator-start 3) + "%%% API" n + (erlang-skel-double-separator-end 3) n + (erlang-skel-separator-start 2) + "%% @doc" n + "%% Creates a gen_statem process which calls Module:init/1 to" n + "%% initialize. To ensure a synchronized start-up procedure, this" n + "%% function does not return until Module:init/1 has returned." n + "%%" n + (erlang-skel-separator-end 2) + "-spec start_link() ->" n> + "{ok, Pid :: pid()} |" n> + "ignore |" n> + "{error, Error :: term()}." n + "start_link() ->" n> + "gen_statem:start_link({local, ?SERVER}, ?MODULE, [], [])." n + n + (erlang-skel-double-separator-start 3) + "%%% gen_statem callbacks" n + (erlang-skel-double-separator-end 3) n + (erlang-skel-separator-start 2) + "%% @private" n + "%% @doc" n + "%% Define the callback_mode() for this callback module." n + (erlang-skel-separator-end 2) + "-spec callback_mode() -> gen_statem:callback_mode()." n + "callback_mode() -> handle_event_function." n + n + (erlang-skel-separator-start 2) + "%% @private" n + "%% @doc" n + "%% Whenever a gen_statem is started using gen_statem:start/[3,4] or" n + "%% gen_statem:start_link/[3,4], this function is called by the new" n + "%% process to initialize." n + (erlang-skel-separator-end 2) + "-spec init(Args :: term()) ->" n> + "{ok, State :: term(), Data :: term()} |" n> + "{ok, State :: term(), Data :: term()," n> + "[gen_statem:action()] | gen_statem:action()} |" n> + "ignore |" n> + "{stop, Reason :: term()}." n + "init([]) ->" n> + "{ok, state_name, #data{}}." n + n + (erlang-skel-separator-start 2) + "%% @private" n + "%% @doc" n + "%% This function is called for every event a gen_statem receives." n + "%%" n + "%% NOTE: If there is no exported function handle_event/4," n + "%% StateName/3 functions are called instead!" n (erlang-skel-separator-end 2) "-spec handle_event(" n> "gen_statem:event_type(), Msg :: term()," n> @@ -965,17 +1067,16 @@ Please see the function `tempo-define-template'.") "-spec code_change(" n> "OldVsn :: term() | {down,term()}," n> "State :: term(), Data :: term(), Extra :: term()) ->" n> - "{gen_statem:callback_mode()," n> - "NewState :: term(), NewData :: term()} |" n> + "{ok, NewState :: term(), NewData :: term()} |" n> "(Reason :: term())." n "code_change(_OldVsn, State, Data, _Extra) ->" n> - "{state_functions, State, Data}." n + "{ok, State, Data}." n n (erlang-skel-double-separator-start 3) "%%% Internal functions" n (erlang-skel-double-separator-end 3) ) - "*The template of a gen_statem. + "*The template of a gen_statem (handle_event/4). Please see the function `tempo-define-template'.") (defvar erlang-skel-wx-object -- cgit v1.2.3 From 401dbd2334cf8f063c9a697eafcd0694e3bbe2fe Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Mon, 25 Jul 2016 15:41:33 +0200 Subject: Rewrite SSL for gen_statem M:callback_mode/0 --- lib/ssl/src/dtls_connection.erl | 13 +++++++------ lib/ssl/src/tls_connection.erl | 15 ++++++++------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/ssl/src/dtls_connection.erl b/lib/ssl/src/dtls_connection.erl index b8be686b99..a0d9982aaa 100644 --- a/lib/ssl/src/dtls_connection.erl +++ b/lib/ssl/src/dtls_connection.erl @@ -65,9 +65,7 @@ hello/3, certify/3, cipher/3, abbreviated/3, %% Handshake states connection/3]). %% gen_statem callbacks --export([terminate/3, code_change/4, format_status/2]). - --define(GEN_STATEM_CB_MODE, state_functions). +-export([callback_mode/0, terminate/3, code_change/4, format_status/2]). %%==================================================================== %% Internal application API @@ -161,12 +159,15 @@ init([Role, Host, Port, Socket, Options, User, CbInfo]) -> State0 = initial_state(Role, Host, Port, Socket, Options, User, CbInfo), try State = ssl_connection:ssl_config(State0#state.ssl_options, Role, State0), - gen_statem:enter_loop(?MODULE, [], ?GEN_STATEM_CB_MODE, init, State) + gen_statem:enter_loop(?MODULE, [], init, State) catch throw:Error -> - gen_statem:enter_loop(?MODULE, [], ?GEN_STATEM_CB_MODE, error, {Error,State0}) + gen_statem:enter_loop(?MODULE, [], error, {Error,State0}) end. +callback_mode() -> + state_functions. + %%-------------------------------------------------------------------- %% State functionsconnection/2 %%-------------------------------------------------------------------- @@ -376,7 +377,7 @@ terminate(Reason, StateName, State) -> %% Description: Convert process state when code is changed %%-------------------------------------------------------------------- code_change(_OldVsn, StateName, State, _Extra) -> - {?GEN_STATEM_CB_MODE, StateName, State}. + {ok, StateName, State}. format_status(Type, Data) -> ssl_connection:format_status(Type, Data). diff --git a/lib/ssl/src/tls_connection.erl b/lib/ssl/src/tls_connection.erl index 9880befa94..eaf866c339 100644 --- a/lib/ssl/src/tls_connection.erl +++ b/lib/ssl/src/tls_connection.erl @@ -68,10 +68,8 @@ hello/3, certify/3, cipher/3, abbreviated/3, %% Handshake states connection/3]). %% gen_statem callbacks --export([terminate/3, code_change/4, format_status/2]). +-export([callback_mode/0, terminate/3, code_change/4, format_status/2]). --define(GEN_STATEM_CB_MODE, state_functions). - %%==================================================================== %% Internal application API %%==================================================================== @@ -169,11 +167,14 @@ init([Role, Host, Port, Socket, Options, User, CbInfo]) -> State0 = initial_state(Role, Host, Port, Socket, Options, User, CbInfo), try State = ssl_connection:ssl_config(State0#state.ssl_options, Role, State0), - gen_statem:enter_loop(?MODULE, [], ?GEN_STATEM_CB_MODE, init, State) + gen_statem:enter_loop(?MODULE, [], init, State) catch throw:Error -> - gen_statem:enter_loop(?MODULE, [], ?GEN_STATEM_CB_MODE, error, {Error, State0}) + gen_statem:enter_loop(?MODULE, [], error, {Error, State0}) end. +callback_mode() -> + state_functions. + %%-------------------------------------------------------------------- %% State functions %%-------------------------------------------------------------------- @@ -457,9 +458,9 @@ format_status(Type, Data) -> %%-------------------------------------------------------------------- code_change(_OldVsn, StateName, State0, {Direction, From, To}) -> State = convert_state(State0, Direction, From, To), - {?GEN_STATEM_CB_MODE, StateName, State}; + {ok, StateName, State}; code_change(_OldVsn, StateName, State, _) -> - {?GEN_STATEM_CB_MODE, StateName, State}. + {ok, StateName, State}. %%-------------------------------------------------------------------- %%% Internal functions -- cgit v1.2.3 From adf04d0c3dbb20e539892a1262078eb5a6538c97 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Mon, 25 Jul 2016 15:41:19 +0200 Subject: Rewrite SSH for gen_statem M:callback_mode/0 --- lib/ssh/src/ssh_connection_handler.erl | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index f9f4c82351..dcb6ff9343 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -60,7 +60,8 @@ ]). %%% Behaviour callbacks --export([handle_event/4, terminate/3, format_status/2, code_change/4]). +-export([callback_mode/0, handle_event/4, terminate/3, + format_status/2, code_change/4]). %%% Exports not intended to be used :). They are used for spawning and tests -export([init_connection_handler/3, % proc_lib:spawn needs this @@ -374,14 +375,12 @@ init_connection_handler(Role, Socket, Opts) -> S -> gen_statem:enter_loop(?MODULE, [], %%[{debug,[trace,log,statistics,debug]} || Role==server], - handle_event_function, {hello,Role}, S) catch _:Error -> gen_statem:enter_loop(?MODULE, [], - handle_event_function, {init_error,Error}, S0) end. @@ -504,6 +503,9 @@ init_ssh_record(Role, Socket, Opts) -> %%% ######## Error in the initialisation #### +callback_mode() -> + handle_event_function. + handle_event(_, _Event, {init_error,Error}, _) -> case Error of {badmatch,{error,enotconn}} -> @@ -1401,12 +1403,12 @@ fmt_stat_rec(FieldNames, Rec, Exclude) -> state_name(), #data{}, term() - ) -> {gen_statem:callback_mode(), state_name(), #data{}}. + ) -> {ok, state_name(), #data{}}. %% . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . code_change(_OldVsn, StateName, State, _Extra) -> - {handle_event_function, StateName, State}. + {ok, StateName, State}. %%==================================================================== -- cgit v1.2.3 From 3a60545091d3075e23c4a7af8c18b3641bb084e2 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Tue, 9 Aug 2016 08:59:51 +0200 Subject: Doc fixes --- lib/stdlib/doc/src/gen_statem.xml | 56 ++++++++++++--------------------- lib/stdlib/src/gen_statem.erl | 8 ++--- system/doc/design_principles/statem.xml | 5 +-- 3 files changed, 27 insertions(+), 42 deletions(-) diff --git a/lib/stdlib/doc/src/gen_statem.xml b/lib/stdlib/doc/src/gen_statem.xml index f18c1087ab..3322571b2c 100644 --- a/lib/stdlib/doc/src/gen_statem.xml +++ b/lib/stdlib/doc/src/gen_statem.xml @@ -119,9 +119,11 @@ erlang:'!' -----> Module:StateName/3

If a callback function fails or returns a bad value, - the gen_statem terminates. However, an exception of class + the gen_statem terminates, unless otherwise stated. + However, an exception of class throw - is not regarded as an error but as a valid return. + is not regarded as an error but as a valid return + from all callback functions.

@@ -917,7 +919,8 @@ handle_event(_, _, State, Data) ->

- To avoid getting a late reply in the caller's + For Timeout =/= infinity, + to avoid getting a late reply in the caller's inbox, this function spawns a proxy process that does the call. A late reply gets delivered to the dead proxy process, hence gets discarded. This is @@ -1288,12 +1291,6 @@ handle_event(_, _, State, Data) -> Module:code_change/4 returns.

-

- This function can use - erlang:throw/1 - to return CallbackMode, just for symmetry reasons. - There should be no actual reason to use this. -

If this function's body does not consist of solely one of two @@ -1368,11 +1365,6 @@ handle_event(_, _, State, Data) -> It is recommended to use an atom as Reason since it will be wrapped in an {error,Reason} tuple.

-

- This function can use - erlang:throw/1 - to return Result or Reason. -

@@ -1432,11 +1424,6 @@ handle_event(_, _, State, Data) -> or ignore; see start_link/3,4.

-

- This function can use - erlang:throw/1 - to return Result. -

This callback is optional, so a callback module does not need @@ -1477,10 +1464,14 @@ handle_event(_, _, State, Data) -> This callback is optional, so a callback module does not need to export it. The gen_statem module provides a default implementation of this function that returns - {State,Data}. If this callback fails, the default - function returns {State,Info}, - where Info informs of the crash but no details, - to hide possibly sensitive data. + {State,Data}. +

+

+ If this callback is exported but fails, + to hide possibly sensitive data, + the default function will instead return {State,Info}, + where Info says nothing but the fact that + format_status/2 has crashed.

This function is called by a gen_statem process when @@ -1541,11 +1532,6 @@ handle_event(_, _, State, Data) -> printed in log files. Another use is to hide sensitive data from being written to the error log.

-

- This function can use - erlang:throw/1 - to return Status. -

@@ -1620,9 +1606,12 @@ handle_event(_, _, State, Data) -> see action().

- These functions can use - erlang:throw/1, - to return the result. + Note the fact that you can use + throw + to return the result, which can be useful. + For example to bail out with throw(keep_state_and_data) + from deep within complex code that is in no position to + return {next_state,State,Data}.

@@ -1695,11 +1684,6 @@ handle_event(_, _, State, Data) -> and an error report is issued using error_logger:format/2.

-

- This function can use - erlang:throw/1 - to return Ignored, which is ignored anyway. -

diff --git a/lib/stdlib/src/gen_statem.erl b/lib/stdlib/src/gen_statem.erl index 6d7b461ee3..a39503cb6b 100644 --- a/lib/stdlib/src/gen_statem.erl +++ b/lib/stdlib/src/gen_statem.erl @@ -1295,7 +1295,7 @@ format_status(Opt, PDict, #{module := Module}, State, Data) -> _:_ -> format_status_default( Opt, State, - "Module:format_status/2 crashed") + atom_to_list(Module) ++ ":format_status/2 crashed") end; false -> format_status_default(Opt, State, Data) @@ -1303,10 +1303,10 @@ format_status(Opt, PDict, #{module := Module}, State, Data) -> %% The default Module:format_status/2 format_status_default(Opt, State, Data) -> - SSD = {State,Data}, + StateData = {State,Data}, case Opt of terminate -> - SSD; + StateData; _ -> - [{data,[{"State",SSD}]}] + [{data,[{"State",StateData}]}] end. diff --git a/system/doc/design_principles/statem.xml b/system/doc/design_principles/statem.xml index 1351997bc1..f785ca9650 100644 --- a/system/doc/design_principles/statem.xml +++ b/system/doc/design_principles/statem.xml @@ -608,7 +608,7 @@ init(Args) -> callback function terminate(shutdown, State, Data).

- In the following example, function terminate/3 + In this example, function terminate/3 locks the door if it is open, so we do not accidentally leave the door open when the supervision tree terminates:

@@ -1217,7 +1217,8 @@ format_status(Opt, [_PDict,State,Data]) -> Module:format_status/2 function. If you do not, a default implementation is used that does the same as this example function without filtering - the Data term, that is, StateData = {State,Data}. + the Data term, that is, StateData = {State,Data}, + in this example containing sensitive information.

-- cgit v1.2.3 From a47d47ac92d080c6f8e4fcba1dd8b8575c5a3f96 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Wed, 17 Aug 2016 10:42:58 +0200 Subject: Clarify error values --- lib/stdlib/src/gen_statem.erl | 38 +++++++++++++++++++++++++----------- lib/stdlib/test/gen_statem_SUITE.erl | 8 ++++---- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/lib/stdlib/src/gen_statem.erl b/lib/stdlib/src/gen_statem.erl index a39503cb6b..7e4fab5365 100644 --- a/lib/stdlib/src/gen_statem.erl +++ b/lib/stdlib/src/gen_statem.erl @@ -582,7 +582,7 @@ init_result(Starter, Parent, ServerRef, Module, Result, Opts) -> proc_lib:init_ack(Starter, ignore), exit(normal); _ -> - Error = {bad_return_value,Result}, + Error = {bad_return_from_init,Result}, proc_lib:init_ack(Starter, {error,Error}), exit(Error) end. @@ -656,11 +656,11 @@ format_status( print_event(Dev, {in,Event}, {Name,_}) -> io:format( - Dev, "*DBG* ~p received ~s~n", + Dev, "*DBG* ~p receive ~s~n", [Name,event_string(Event)]); print_event(Dev, {out,Reply,{To,_Tag}}, {Name,_}) -> io:format( - Dev, "*DBG* ~p sent ~p to ~p~n", + Dev, "*DBG* ~p send ~p to ~p~n", [Name,Reply,To]); print_event(Dev, {Tag,Event,NextState}, {Name,State}) -> StateString = @@ -992,7 +992,9 @@ loop_event_result( State, Data, P, Event, State, Actions); _ -> terminate( - error, {bad_return_value,Result}, ?STACKTRACE(), + error, + {bad_return_from_state_function,Result}, + ?STACKTRACE(), Debug, S, [Event|Events], State, Data, P) end. @@ -1029,7 +1031,9 @@ loop_event_actions( Postpone, Hibernate, Timeout, NextEvents); false -> terminate( - error, {bad_action,Action}, ?STACKTRACE(), + error, + {bad_action_from_state_function,Action}, + ?STACKTRACE(), Debug, S, [Event|Events], State, NewData, P) end; {next_event,Type,Content} -> @@ -1042,7 +1046,9 @@ loop_event_actions( [{Type,Content}|NextEvents]); false -> terminate( - error, {bad_action,Action}, ?STACKTRACE(), + error, + {bad_action_from_state_function,Action}, + ?STACKTRACE(), Debug, S, [Event|Events], State, NewData, P) end; %% Actions that set options @@ -1053,7 +1059,9 @@ loop_event_actions( NewPostpone, Hibernate, Timeout, NextEvents); {postpone,_} -> terminate( - error, {bad_action,Action}, ?STACKTRACE(), + error, + {bad_action_from_state_function,Action}, + ?STACKTRACE(), Debug, S, [Event|Events], State, NewData, P); postpone -> loop_event_actions( @@ -1067,7 +1075,9 @@ loop_event_actions( Postpone, NewHibernate, Timeout, NextEvents); {hibernate,_} -> terminate( - error, {bad_action,Action}, ?STACKTRACE(), + error, + {bad_action_from_state_function,Action}, + ?STACKTRACE(), Debug, S, [Event|Events], State, NewData, P); hibernate -> loop_event_actions( @@ -1086,7 +1096,9 @@ loop_event_actions( Postpone, Hibernate, NewTimeout, NextEvents); {timeout,_,_} -> terminate( - error, {bad_action,Action}, ?STACKTRACE(), + error, + {bad_action_from_state_function,Action}, + ?STACKTRACE(), Debug, S, [Event|Events], State, NewData, P); infinity -> % Clear timer - it will never trigger loop_event_actions( @@ -1101,7 +1113,9 @@ loop_event_actions( Postpone, Hibernate, NewTimeout, NextEvents); _ -> terminate( - error, {bad_action,Action}, ?STACKTRACE(), + error, + {bad_action_from_state_function,Action}, + ?STACKTRACE(), Debug, S, [Event|Events], State, NewData, P) end; %% @@ -1173,7 +1187,9 @@ do_reply_then_terminate( NewDebug, S, Q, State, Data, P, Rs); _ -> terminate( - error, {bad_action,R}, ?STACKTRACE(), + error, + {bad_reply_action_from_state_function,R}, + ?STACKTRACE(), Debug, S, Q, State, Data, P) end. diff --git a/lib/stdlib/test/gen_statem_SUITE.erl b/lib/stdlib/test/gen_statem_SUITE.erl index d90119de54..1d1417c2e6 100644 --- a/lib/stdlib/test/gen_statem_SUITE.erl +++ b/lib/stdlib/test/gen_statem_SUITE.erl @@ -460,10 +460,10 @@ abnormal2(Config) -> {ok,Pid} = gen_statem:start_link(?MODULE, start_arg(Config, []), []), %% bad return value in the gen_statem loop - {{bad_return_value,badreturn},_} = + {{bad_return_from_state_function,badreturn},_} = ?EXPECT_FAILURE(gen_statem:call(Pid, badreturn), Reason), receive - {'EXIT',Pid,{bad_return_value,badreturn}} -> ok + {'EXIT',Pid,{bad_return_from_state_function,badreturn}} -> ok after 5000 -> ct:fail(gen_statem_did_not_die) end, @@ -709,7 +709,7 @@ error_format_status(Config) -> gen_statem:start( ?MODULE, start_arg(Config, {data,Data}), []), %% bad return value in the gen_statem loop - {{bad_return_value,badreturn},_} = + {{bad_return_from_state_function,badreturn},_} = ?EXPECT_FAILURE(gen_statem:call(Pid, badreturn), Reason), receive {error,_, @@ -717,7 +717,7 @@ error_format_status(Config) -> "** State machine"++_, [Pid,{{call,_},badreturn}, {formatted,idle,Data}, - error,{bad_return_value,badreturn}|_]}} -> + error,{bad_return_from_state_function,badreturn}|_]}} -> ok; Other when is_tuple(Other), element(1, Other) =:= error -> error_logger_forwarder:unregister(), -- cgit v1.2.3 From b6541d133f166a3f28cc3b2daf14c59024312c60 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Wed, 17 Aug 2016 14:29:43 +0200 Subject: Handle exceptions in init/1 and callback_mode/0 --- lib/stdlib/src/gen_statem.erl | 119 +++++++++++++++++++++++++----------------- 1 file changed, 72 insertions(+), 47 deletions(-) diff --git a/lib/stdlib/src/gen_statem.erl b/lib/stdlib/src/gen_statem.erl index 7e4fab5365..eee7d60e9d 100644 --- a/lib/stdlib/src/gen_statem.erl +++ b/lib/stdlib/src/gen_statem.erl @@ -557,9 +557,15 @@ init_it(Starter, Parent, ServerRef, Module, Args, Opts) -> Result -> init_result(Starter, Parent, ServerRef, Module, Result, Opts); Class:Reason -> + Stacktrace = erlang:get_stacktrace(), + Name = gen:get_proc_name(ServerRef), gen:unregister_name(ServerRef), proc_lib:init_ack(Starter, {error,Reason}), - erlang:raise(Class, Reason, erlang:get_stacktrace()) + error_info( + Class, Reason, Stacktrace, + #{name => Name, callback_mode => undefined}, + [], [], undefined), + erlang:raise(Class, Reason, Stacktrace) end. %%--------------------------------------------------------------------------- @@ -582,8 +588,14 @@ init_result(Starter, Parent, ServerRef, Module, Result, Opts) -> proc_lib:init_ack(Starter, ignore), exit(normal); _ -> + Name = gen:get_proc_name(ServerRef), + gen:unregister_name(ServerRef), Error = {bad_return_from_init,Result}, proc_lib:init_ack(Starter, {error,Error}), + error_info( + error, Error, ?STACKTRACE(), + #{name => Name, callback_mode => undefined}, + [], [], undefined), exit(Error) end. @@ -835,31 +847,6 @@ loop_events_done(Parent, Debug, S, Timer, State, Data, P, Hibernate) -> timer := Timer}, loop(Parent, Debug, NewS). -loop_event( - Parent, Debug, - #{callback_mode := undefined, - module := Module} = S, - Events, - State, Data, P, Event, Hibernate) -> - %% Cache the callback_mode() value - case - try Module:callback_mode() - catch - Result -> Result - end - of - CallbackMode -> - case callback_mode(CallbackMode) of - true -> - loop_event( - Parent, Debug, - S#{callback_mode := CallbackMode}, - Events, - State, Data, P, Event, Hibernate); - false -> - error({callback_mode,CallbackMode}) - end - end; loop_event( Parent, Debug, #{callback_mode := CallbackMode, @@ -878,22 +865,36 @@ loop_event( %% try case CallbackMode of + undefined -> + Module:callback_mode(); state_functions -> - Module:State(Type, Content, Data); + erlang:apply(Module, State, [Type,Content,Data]); handle_event_function -> Module:handle_event(Type, Content, State, Data) - end of + end + of + Result when CallbackMode =:= undefined -> + loop_event_callback_mode( + Parent, Debug, S, Events, State, Data, P, Event, Result); Result -> loop_event_result( Parent, Debug, S, Events, State, Data, P, Event, Result) catch + Result when CallbackMode =:= undefined -> + loop_event_callback_mode( + Parent, Debug, S, Events, State, Data, P, Event, Result); Result -> loop_event_result( Parent, Debug, S, Events, State, Data, P, Event, Result); - error:badarg when CallbackMode =:= state_functions -> + error:badarg -> case erlang:get_stacktrace() of - [{erlang,apply,[Module,State,_],_}|Stacktrace] -> - Args = [Type,Content,Data], + [{erlang,apply, + [Module,State,[Type,Content,Data]=Args], + _} + |Stacktrace] + when CallbackMode =:= state_functions -> + %% We get here e.g if apply fails + %% due to State not being an atom terminate( error, {undef_state_function,{Module,State,Args}}, @@ -905,24 +906,29 @@ loop_event( Debug, S, [Event|Events], State, Data, P) end; error:undef -> - %% Process an undef to check for the simple mistake + %% Process undef to check for the simple mistake %% of calling a nonexistent state function + %% to make the undef more precise case erlang:get_stacktrace() of - [{Module,State, - [Type,Content,Data]=Args, - _} + [{Module,callback_mode,[]=Args,_} |Stacktrace] - when CallbackMode =:= state_functions -> + when CallbackMode =:= undefined -> + terminate( + error, + {undef_callback,{Module,callback_mode,Args}}, + Stacktrace, + Debug, S, [Event|Events], State, Data, P); + [{Module,State,[Type,Content,Data]=Args,_} + |Stacktrace] + when CallbackMode =:= state_functions -> terminate( error, {undef_state_function,{Module,State,Args}}, Stacktrace, Debug, S, [Event|Events], State, Data, P); - [{Module,handle_event, - [Type,Content,State,Data]=Args, - _} + [{Module,handle_event,[Type,Content,State,Data]=Args,_} |Stacktrace] - when CallbackMode =:= handle_event_function -> + when CallbackMode =:= handle_event_function -> terminate( error, {undef_state_function,{Module,handle_event,Args}}, @@ -940,6 +946,25 @@ loop_event( Debug, S, [Event|Events], State, Data, P) end. +%% Interpret callback_mode() result +loop_event_callback_mode( + Parent, Debug, S, Events, State, Data, P, Event, CallbackMode) -> + case callback_mode(CallbackMode) of + true -> + Hibernate = false, % We have already GC:ed recently + loop_event( + Parent, Debug, + S#{callback_mode := CallbackMode}, + Events, + State, Data, P, Event, Hibernate); + false -> + terminate( + error, + {bad_return_from_callback_mode,CallbackMode}, + ?STACKTRACE(), + Debug, S, [Event|Events], State, Data, P) + end. + %% Interpret all callback return variants loop_event_result( Parent, Debug, S, Events, State, Data, P, Event, Result) -> @@ -1208,8 +1233,9 @@ terminate( C:R -> ST = erlang:get_stacktrace(), error_info( - C, R, ST, Debug, S, Q, P, + C, R, ST, S, Q, P, format_status(terminate, get(), S, State, Data)), + sys:print_log(Debug), erlang:raise(C, R, ST) end, case Reason of @@ -1218,8 +1244,9 @@ terminate( {shutdown,_} -> ok; _ -> error_info( - Class, Reason, Stacktrace, Debug, S, Q, P, - format_status(terminate, get(), S, State, Data)) + Class, Reason, Stacktrace, S, Q, P, + format_status(terminate, get(), S, State, Data)), + sys:print_log(Debug) end, case Stacktrace of [] -> @@ -1229,7 +1256,7 @@ terminate( end. error_info( - Class, Reason, Stacktrace, Debug, + Class, Reason, Stacktrace, #{name := Name, callback_mode := CallbackMode}, Q, P, FmtData) -> {FixedReason,FixedStacktrace} = @@ -1296,9 +1323,7 @@ error_info( case FixedStacktrace of [] -> []; _ -> [FixedStacktrace] - end), - sys:print_log(Debug), - ok. + end). %% Call Module:format_status/2 or return a default value -- cgit v1.2.3 From 9b1179d8c8c411a245c59fc7092e4a7a98f76663 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Thu, 18 Aug 2016 11:18:37 +0200 Subject: Improve sys debug --- lib/stdlib/src/gen_statem.erl | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/stdlib/src/gen_statem.erl b/lib/stdlib/src/gen_statem.erl index eee7d60e9d..3b3477b282 100644 --- a/lib/stdlib/src/gen_statem.erl +++ b/lib/stdlib/src/gen_statem.erl @@ -666,14 +666,14 @@ format_status( %% them, not as the real erlang messages. Use trace for that. %%--------------------------------------------------------------------------- -print_event(Dev, {in,Event}, {Name,_}) -> +print_event(Dev, {in,Event}, {Name,State}) -> io:format( - Dev, "*DBG* ~p receive ~s~n", - [Name,event_string(Event)]); -print_event(Dev, {out,Reply,{To,_Tag}}, {Name,_}) -> + Dev, "*DBG* ~p receive ~s in state ~p~n", + [Name,event_string(Event),State]); +print_event(Dev, {out,Reply,{To,_Tag}}, {Name,State}) -> io:format( - Dev, "*DBG* ~p send ~p to ~p~n", - [Name,Reply,To]); + Dev, "*DBG* ~p send ~p to ~p from state ~p~n", + [Name,Reply,To,State]); print_event(Dev, {Tag,Event,NextState}, {Name,State}) -> StateString = case NextState of @@ -1064,8 +1064,10 @@ loop_event_actions( {next_event,Type,Content} -> case event_type(Type) of true -> + NewDebug = + sys_debug(Debug, S, State, {in,{Type,Content}}), loop_event_actions( - Parent, Debug, S, Events, + Parent, NewDebug, S, Events, State, NewData, P, Event, NextState, Actions, Postpone, Hibernate, Timeout, [{Type,Content}|NextEvents]); -- cgit v1.2.3 From 88e52188bf5813e742f46767a38ea8234cdc53e5 Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Thu, 18 Aug 2016 11:19:12 +0200 Subject: Include trap_exit in server skeletons --- lib/tools/emacs/erlang-skels.el | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/tools/emacs/erlang-skels.el b/lib/tools/emacs/erlang-skels.el index 43763d2c8a..0284c9d686 100644 --- a/lib/tools/emacs/erlang-skels.el +++ b/lib/tools/emacs/erlang-skels.el @@ -499,6 +499,7 @@ Please see the function `tempo-define-template'.") "%% {stop, Reason}" n (erlang-skel-separator-end 2) "init([]) ->" n> + "process_flag(trap_exit, true)," n> "{ok, #state{}}." n n (erlang-skel-separator-start 2) @@ -742,6 +743,7 @@ Please see the function `tempo-define-template'.") "%% {stop, StopReason}" n (erlang-skel-separator-end 2) "init([]) ->" n> + "process_flag(trap_exit, true)," n> "{ok, state_name, #state{}}." n n (erlang-skel-separator-start 2) @@ -919,6 +921,7 @@ Please see the function `tempo-define-template'.") "ignore |" n> "{stop, Reason :: term()}." n "init([]) ->" n> + "process_flag(trap_exit, true)," n> "{ok, state_name, #data{}}." n n (erlang-skel-separator-start 2) @@ -1029,6 +1032,7 @@ Please see the function `tempo-define-template'.") "ignore |" n> "{stop, Reason :: term()}." n "init([]) ->" n> + "process_flag(trap_exit, true)," n> "{ok, state_name, #data{}}." n n (erlang-skel-separator-start 2) -- cgit v1.2.3