[erlang-questions] gen_statem and multiple timeout vs 1

[Raimo Niskanen]

On Thu, Oct 06, 2016 at 03:46:55PM +0000, Vans S wrote:
> Okay going to take another attempt at it. In evented mode.
> 
> Some points before we get into it. gen_statem fits here because it ensures 2 key points.
> 
> #1 If we get any message while in waiting_socket state, we should not process it / crash / drop it.
> #2 We need to transition out of waiting_socket state, before processing future messages.
> 
> This code might be off as I did not test it, so treat as pseudo code.
> 

Your mail client really messed the code up so I had to reformat it
before reading it...


init() ->
    {ok, waiting_socket, #{},
     [{state_timeout, 2000, no_socket_passed}]}.

%% Timeouts
%% Crash here or handle ourselves
handle_event(state_timeout, no_socket_passed, waiting_socket, D) ->
    {stop, {shutdown, tcp_closed}, D};
%% Crash here or handle, we should NEVER get any message here
handle_event(_, _, waiting_socket, D) ->
    {stop, {shutdown, tcp_closed}, D};
handle_event(named_timeout, endpoint_timeout, _, D) ->
    {stop, {shutdown, tcp_closed}, D};

%% Basic socket ops
handle_event(info, {pass_socket, Socket}, waiting_socket, D) ->
    {next_state, established, D#{socket=> Socket},
      [{named_timeout, 15000, keep_alive},
       {named_timeout, 120000, endpoint_timeout}]};

handle_event(named_timeout, keep_alive, S, D) ->
    send_keepalive(),
    {next_state, S, D, [{named_timeout, 15000, keep_alive}]};

%% Login chain
%% Moves our state to 'ingame' if success
%% Not implemented in example

%% Ingame
handle_event(info, {tcp, Socket, Bin}, ingame, D) ->
   case proc_on_bin(Bin) of
      {build, Building, X, Y} ->
           {BuildUUID, BuildTime} = get_build_time(Building),
           register_build_uuid(BuildUUID, Building, X, Y),        
           %% Make some helper functions to make this cleaner
           % A cleaner option is to send a message to mailbox
           % after parsing packet
           {next_state, ingame, D,
            [{named_timeout, BuildTime, {built, BuildUUID}},
	     {named_timeout, 15000, keep_alive},
	     {named_timeout, 120000, endpoint_timeout}]};
      {cancel_build, BuildUUID} ->
           unregister_build_uuid(BuildUUID),
           {next_state, ingame, D, [
               {named_timeout, infinity, BuildUUID},
               {named_timeout, 15000, keep_alive},
               {named_timeout, 120000, endpoint_timeout}
           ]}
   end;
handle_event(named_timeout, {built, BuildUUID}, ingame, D) ->
    Socket = maps:get(socket, D),
    notify_client_building_upgraded(Socket, BuildUUID),
    {next_state, ingame, D};

> 
> 
> To me this named_timeout really fits this model, keeps the code clean, keeps it all in one place. state_timeout helps us correctly transition our states from outgame/ingame/lobby/etc.

I also like the look of the named timeouts in that code.  It is quite readable.

How to bump a timer is not solved, but I think needs to be solved for
state_timeout anyway...

Solving it with erlang:start_timer/3,4 might look like this
(selected parts):


handle_event(
  internal, {timeout,TimerRef,keep_alive, S,
  #{named_timeout := #{keep_alive := TimerRef}} = D) ->
    send_keepalive(),
    {next_state, S, named_timeout(1500, keep_alive, D)};

handle_event(info, {tcp,Socket,Bin}, ingame, D) ->
    ...
    {next_state, ingame,
     named_timeout(
	 BuildTime, {built,BuildUUID},
         named_timeout(
	     15000, keep_alive,
	     named_timeout, 120000, endpoint_timeout, D)};
handle_event(
  internal, {timeout,TimerRef,{built,BuildUUID} = Built}, S, D) ->
    case D of
        #{named_timeout := #{Built := TimerRef}} ->
	    Socket = maps:get(socket, D),
	    notify_client_building_upgraded(Socked, BuildUUID),
	    {next_state, ingame, named_timeout(TimerRef, Built, D)};
	_ ->
	    %% What to do with unexpected timeout...
    end;


The named_timeout/3 helper function remains to be written, matching
the timer event is much messier, and you need to remember to remove
the TimerRef from the state when you get the timeout event...

So, named_timeout would make timer handling easier.
The question is if it will be flexible enough?

> Possibly we could have 1 process for loggedout/outgame/lobby and 1 process for ingame, so we can chat while playing.
> The original 'timeout' seems out of place, only use case I see for it is if the process is idle to hibernate it, or to kill the process after a certain time, or other similar.
> 
> So to summarize, gen_statem allows us to guarantee state transitions even if other messages arrive to mailbox.  Thus if we were to implement this code using processes or gen_server we
> would be recreating a lot of gen_statem.  Using the handle_event_function callback mode allows us to have this kind of flexible gen_statem.
> 
> This is my final argument in support of named_timeout.  Thank you for your attention.

Your case is certainly worth considering.  We'll start with completing
state_timeout and then we will see...


-- 

/ Raimo Niskanen, Erlang/OTP, Ericsson AB