From 26b3c7d60d52d8a7be006b06d856bb0f7276e77a Mon Sep 17 00:00:00 2001 From: Raimo Niskanen Date: Thu, 21 Apr 2016 15:58:52 +0200 Subject: Modify code_change/4 to return CallbackMode Also move check of non-atom states in callback mode state_functions to where the state function is called. This gives homogenous diagnostics for state functions, code_change/4 and system_replace_state StateFun. Irregularities pointed out by James Fish. --- lib/stdlib/doc/src/gen_statem.xml | 71 ++++++++++++++++++++++-------------- lib/stdlib/src/gen_statem.erl | 60 +++++++++++++++--------------- lib/stdlib/test/gen_statem_SUITE.erl | 8 ++-- 3 files changed, 79 insertions(+), 60 deletions(-) (limited to 'lib/stdlib') diff --git a/lib/stdlib/doc/src/gen_statem.xml b/lib/stdlib/doc/src/gen_statem.xml index c21398d64d..adca3673be 100644 --- a/lib/stdlib/doc/src/gen_statem.xml +++ b/lib/stdlib/doc/src/gen_statem.xml @@ -32,8 +32,9 @@ Generic State Machine Behaviour

- A behaviour module for implementing a state machine. - Two callback modes are supported. One for a finite state + A behaviour module for implementing a state machine. Two + callback modes + are supported. One for a finite state machine like gen_fsm that requires the state to be an atom and use that state as the name of the callback function for a particular state, @@ -89,13 +90,13 @@ erlang:'!' -----> Module:StateName/3 The "state function" for a specific state in a gen_statem is the callback function that is called - for all events in this state, and is selected depending on - callback_mode + for all events in this state, and is selected depending on which + callback mode that the implementation specifies when the the server starts.

- When - callback_mode + When the + callback mode is state_functions, the state has to be an atom and is used as the state function name. See @@ -110,7 +111,8 @@ erlang:'!' -----> Module:StateName/3 makes the state name terminate unusable.

- When callback_mode + When the + callback mode is handle_event_function the state can be any term and the state function name is @@ -208,9 +210,7 @@ erlang:'!' -----> Module:StateName/3

This example shows a simple pushbutton model for a toggling pushbutton implemented with - - callback_mode() - + callback mode state_functions. You can push the button and it replies if it went on or off, and you can ask for a count of how many times it has been @@ -226,6 +226,7 @@ erlang:'!' -----> Module:StateName/3 -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. @@ -242,12 +243,12 @@ stop() -> terminate(_Reason, _State, _Data) -> void. code_change(_Vsn, State, Data, _Extra) -> - {ok,State,Data}. + {callback_mode(),State,Data}. init([]) -> %% Set the callback mode and initial state + data. %% Data is used only as a counter. State = off, Data = 0, - {state_functions,State,Data}. + {callback_mode(),State,Data}. %%% State functions @@ -297,9 +298,7 @@ ok

And just to compare styles here is the same example using - - callback_mode - + callback mode state_functions, or rather here is code to replace from the init/1 function of the pushbutton.erl example file above: @@ -453,10 +452,8 @@ handle_event(_, _, State, Data) ->

- If - - callback_mode - + If the + callback mode is state_functions, the state has to be of this type.

@@ -499,7 +496,7 @@ handle_event(_, _, State, Data) ->

- The callback_mode is selected when starting the + The callback mode is selected when starting the gen_statem using the return value from Module:init/1 or when calling @@ -1277,7 +1274,7 @@ handle_event(_, _, State, Data) -> return {CallbackMode,State,Data} or {CallbackMode,State,Data,Actions}. CallbackMode selects the - callback_mode(). + callback mode. of the gen_statem. State is the initial state() @@ -1344,8 +1341,8 @@ handle_event(_, _, State, Data) -> Whenever a gen_statem receives an event from call/2, cast/2 or - as a normal process message one of these functions is called. - If callback_mode + as a normal process message one of these functions is called. If + callback mode is state_functions then Module:StateName/3 is called, and if it is handle_event_function then Module:handle_event/4 is called. @@ -1468,7 +1465,11 @@ handle_event(_, _, State, Data) ->   Vsn = term() OldState = NewState = term() Extra = term() - Result = {ok,NewState,NewData} | Reason + Result = {NewCallbackMode,NewState,NewData} | Reason + + NewCallbackMode = + callback_mode() + OldState = NewState = state() @@ -1484,8 +1485,9 @@ handle_event(_, _, State, Data) -> This function is called by a gen_statem when it should update its internal state during a release upgrade/downgrade, that is when the instruction {update,Module,Change,...} - where Change={advanced,Extra} is given in - the appup file. See + where Change={advanced,Extra} is given in the + appup + file. See OTP Design Principles @@ -1499,6 +1501,21 @@ 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 + since the new code surely knows what callback mode + it needs, but for a downgrade this function will have to + know from the Extra argument that comes from the + appup + file what callback mode the old code did use. + It may also be possible to figure this out + from the {down,Vsn} argument since Vsn + in effect defines the old callback module version. +

+

OldState and OldData is the internal state of the gen_statem. @@ -1510,7 +1527,7 @@ handle_event(_, _, State, Data) ->

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

If the function returns Reason, the ongoing diff --git a/lib/stdlib/src/gen_statem.erl b/lib/stdlib/src/gen_statem.erl index b3a149a569..c85c521d8e 100644 --- a/lib/stdlib/src/gen_statem.erl +++ b/lib/stdlib/src/gen_statem.erl @@ -219,7 +219,9 @@ OldState :: state(), OldData :: data(), Extra :: term()) -> - {ok, NewState :: state(), NewData :: data()}. + {NewCallbackMode :: callback_mode(), + NewState :: state(), + NewData :: data()}. %% Format the callback module state in some sensible that is %% often condensed way. For StatusOption =:= 'normal' the perferred @@ -491,17 +493,10 @@ enter_loop(Module, Opts, CallbackMode, State, Data, Server_or_Actions) -> Actions :: [action()] | action()) -> no_return(). enter_loop(Module, Opts, CallbackMode, State, Data, Server, Actions) -> - case is_atom(Module) andalso callback_mode(CallbackMode) of - true -> - Parent = gen:get_parent(), - enter( - Module, Opts, CallbackMode, State, Data, Server, - Actions, Parent); - false -> - error( - badarg, - [Module,Opts,CallbackMode,State,Data,Server,Actions]) - end. + 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). %%--------------------------------------------------------------------------- %% API helpers @@ -580,15 +575,13 @@ init_result(Starter, Parent, ServerRef, Module, Result, Opts) -> case Result of {CallbackMode,State,Data} -> case callback_mode(CallbackMode) of - true - when CallbackMode =:= state_functions, is_atom(State); - CallbackMode =/= state_functions -> + true -> proc_lib:init_ack(Starter, {ok,self()}), enter( Module, Opts, CallbackMode, State, Data, ServerRef, [], Parent); false -> - Error = {bad_return_value,Result}, + Error = {callback_mode,CallbackMode}, proc_lib:init_ack(Starter, {error,Error}), exit(Error) end; @@ -600,7 +593,7 @@ init_result(Starter, Parent, ServerRef, Module, Result, Opts) -> Module, Opts, CallbackMode, State, Data, ServerRef, Actions, Parent); false -> - Error = {bad_return_value,Result}, + Error = {callback_mode,CallbackMode}, proc_lib:init_ack(Starter, {error,Error}), exit(Error) end; @@ -638,11 +631,12 @@ system_code_change( Result -> Result end of - {ok,NewState,NewData} -> - {ok, - S#{ - state := NewState, - data := NewData}}; + {NewCallbackMode,NewState,NewData} -> + callback_mode(NewCallbackMode) orelse + error({callback_mode,NewCallbackMode}), + {ok,S#{state := NewState, data := NewData}}; + {ok,_} = Error -> + error({case_clause,Error}); Error -> Error end. @@ -825,6 +819,18 @@ loop_events( Result -> loop_event_result( Parent, Debug, S, Events, Event, Result); + error:badarg when CallbackMode =:= state_functions -> + case erlang:get_stacktrace() of + [{erlang,apply,[Module,State,_],_}|Stacktrace] -> + Args = [Type,Content,Data], + terminate( + error, + {undef_state_function,{Module,State,Args}}, + Stacktrace, + Debug, S, Q); + Stacktrace -> + terminate(error, badarg, Stacktrace, Debug, S, Q) + end; error:undef -> %% Process an undef to check for the simple mistake %% of calling a nonexistent state function @@ -861,7 +867,7 @@ loop_events( %% Interpret all callback return variants loop_event_result( Parent, Debug, - #{callback_mode := CallbackMode, state := State, data := Data} = S, + #{state := State, data := Data} = S, Events, Event, Result) -> case Result of stop -> @@ -887,15 +893,11 @@ loop_event_result( exit, Reason, ?STACKTRACE(), Debug, NewS, Q, Replies), %% Since we got back here Replies was bad terminate(Class, NewReason, Stacktrace, NewDebug, NewS, Q); - {next_state,NextState,NewData} - when CallbackMode =:= state_functions, is_atom(NextState); - CallbackMode =/= state_functions -> + {next_state,NextState,NewData} -> loop_event_actions( Parent, Debug, S, Events, Event, State, NextState, NewData, []); - {next_state,NextState,NewData,Actions} - when CallbackMode =:= state_functions, is_atom(NextState); - CallbackMode =/= state_functions -> + {next_state,NextState,NewData,Actions} -> loop_event_actions( Parent, Debug, S, Events, Event, State, NextState, NewData, Actions); diff --git a/lib/stdlib/test/gen_statem_SUITE.erl b/lib/stdlib/test/gen_statem_SUITE.erl index 27fb7d9650..3deb5fd986 100644 --- a/lib/stdlib/test/gen_statem_SUITE.erl +++ b/lib/stdlib/test/gen_statem_SUITE.erl @@ -637,9 +637,9 @@ code_change(Config) -> {ok,Pid} = gen_statem:start(?MODULE, start_arg(Config, []), []), {idle,data} = sys:get_state(Pid), sys:suspend(Pid), - sys:change_code(Pid, ?MODULE, old_vsn, extra), + sys:change_code(Pid, ?MODULE, old_vsn, state_functions), sys:resume(Pid), - {idle,{old_vsn,data,extra}} = sys:get_state(Pid), + {idle,{old_vsn,data,state_functions}} = sys:get_state(Pid), stop_it(Pid). call_format_status(Config) -> @@ -1561,8 +1561,8 @@ wrap_result(Result) -> -code_change(OldVsn, State, Data, Extra) -> - {ok,State,{OldVsn,Data,Extra}}. +code_change(OldVsn, State, Data, CallbackMode) -> + {CallbackMode,State,{OldVsn,Data,CallbackMode}}. format_status(terminate, [_Pdict,State,Data]) -> {formatted,State,Data}; -- cgit v1.2.3