Merge pull request erlang#9615 from juhlig/gen_server_plain_timeout_fix OTP-19537

Fix `gen_server` plain timeouts being canceled by `system` messages

Author: RaimoNiskanen

lib/stdlib/src/gen_server.erl

@@ -2256,7 +2256,7 @@ loop(ServerData, State, {continue, Continue} = Msg, Debug) ->
 loop(ServerData, State, LoopAction, Debug) ->
     case LoopAction of
         {timeout_zero, TimeoutMsg} ->
-            decode_msg(ServerData, State, TimeoutMsg, Debug, [], false);
+            decode_msg(ServerData, State, infinity, Debug, [], TimeoutMsg);
         {timeout, Timer, HibInf} ->
             loop(ServerData, State, HibInf, Debug, Timer);
         HibT ->
@@ -2267,46 +2267,39 @@ loop(ServerData, State, hibernate, Debug, Timer) ->
     receive
 	Msg ->
 	    erlang:garbage_collect(),
-	    decode_msg(ServerData, State, Msg, Debug, Timer, true)
+	    decode_msg(ServerData, State, hibernate, Debug, Timer, Msg)
     after 0 ->
-	loop_hibernate(ServerData, State, Debug, Timer)
+	loop_hibernate(ServerData, State, Timer, Debug)
     end;
 %%
-loop(#server_data{hibernate_after = HibernateAfterTimeout} = ServerData, State, infinity, Debug, Timer) ->
+loop(ServerData, State, infinity, Debug, Timer) ->
     receive
 	Msg ->
-	    decode_msg(ServerData, State, Msg, Debug, Timer, false)
-    after HibernateAfterTimeout ->
-	loop_hibernate(ServerData, State, Debug, Timer)
+	    decode_msg(ServerData, State, infinity, Debug, Timer, Msg)
+    after ServerData#server_data.hibernate_after ->
+	loop_hibernate(ServerData, State, Timer, Debug)
     end;
 %%
 loop(ServerData, State, Time, Debug, Timer)
   when ?is_rel_timeout(Time) ->
-    Msg = receive
-	      Input ->
-		  Input
-	  after Time ->
-	      timeout
-	  end,
-    decode_msg(ServerData, State, Msg, Debug, Timer, false).
-
-loop_hibernate(ServerData, State, Debug, Timer) ->
+    receive
+        Msg ->
+            decode_msg(ServerData, State, Time, Debug, Timer, Msg)
+    after Time  ->
+            decode_msg(ServerData, State, infinity, Debug, Timer, timeout)
+    end.
+
+loop_hibernate(ServerData, State, Timer, Debug) ->
     erlang:hibernate(),
-    loop_wakeup(ServerData, State, Debug, Timer).
+    loop_wakeup(ServerData, State, Timer, Debug).
 
-loop_wakeup(ServerData, State, Debug, Timer) ->
+loop_wakeup(ServerData, State, Timer, Debug) ->
     receive
 	Msg ->
-	    decode_msg(update_callback_cache(ServerData), State, Msg, Debug, Timer, true)
+	    decode_msg(
+              update_callback_cache(ServerData), State, hibernate, Debug, Timer, Msg)
     end.
 
-loop_continue(ServerData, State, Hib, Debug, Timer) ->
-    Action = case Hib of
-		 true -> hibernate;
-		 false -> infinity
-	     end,
-    loop(ServerData, State, Action, Debug, Timer).
-
 cancel_timer([]) ->
     ok;
 cancel_timer([TRef | _]) ->
@@ -2332,17 +2325,17 @@ update_callback_cache(#server_data{module = Mod} = ServerData) ->
       handle_info     = fun Mod:handle_info/2,
       handle_continue = fun Mod:handle_continue/2}.
 
-decode_msg(#server_data{parent = Parent, tag = Tag} = ServerData, State, Msg, Debug, Timer, Hib) ->
+decode_msg(#server_data{parent = Parent, tag = Tag} = ServerData, State, HibT, Debug, Timer, Msg) ->
     case Msg of
         {system, From, Req} ->
             sys:handle_system_msg(Req, From, Parent, ?MODULE, Debug,
-                                  [ServerData, State, Timer, Hib], Hib);
+                                  [ServerData, State, HibT, Timer], HibT =:= hibernate);
         {'EXIT', Parent, Reason} ->
             terminate(ServerData, State, Msg, undefined, Reason, ?STACKTRACE(), Debug);
 	{timeout, TRef, Tag} when TRef =:= hd(Timer) ->
             decode_msg(ServerData, State, tl(Timer), Debug);
 	{timeout, _Stale, Tag} ->
-	    loop_continue(ServerData, State, Hib, Debug, Timer);
+	    loop(ServerData, State, HibT, Debug, Timer);
         _ ->
 	    cancel_timer(Timer),
             decode_msg(ServerData, State, Msg, Debug)
@@ -2613,8 +2606,9 @@ listify(Opt) ->
 %% Callback functions for system messages handling.
 %%-----------------------------------------------------------------
 -doc false.
-system_continue(Parent, Debug, [#server_data{parent=Parent} = ServerData, State, Timer, Hib]) ->
-    loop_continue(update_callback_cache(ServerData), State, Hib, Debug, Timer).
+system_continue(
+  Parent, Debug, [#server_data{parent=Parent} = ServerData, State, HibT, Timer]) ->
+    loop(update_callback_cache(ServerData), State, HibT, Debug, Timer).
 
 -doc false.
 -spec system_terminate(_, _, _, [_]) -> no_return().

lib/stdlib/test/gen_server_SUITE.erl

@@ -33,6 +33,7 @@
          send_request_check_reqid_collection/1,
          cast/1, cast_fast/1, continue/1, info/1, abcast/1,
 	 handle_event_timeout/1, handle_event_timeout_zero/1,
+	 handle_event_timeout_plain/1,
          multicall/1, multicall_down/1, multicall_remote/1,
          multicall_remote_old1/1, multicall_remote_old2/1,
          multicall_recv_opt_success/1,
@@ -93,6 +94,7 @@ all() ->
      send_request_receive_reqid_collection, send_request_wait_reqid_collection,
      send_request_check_reqid_collection, cast, cast_fast, info, abcast,
      continue, handle_event_timeout, handle_event_timeout_zero,
+     handle_event_timeout_plain,
      {group, multi_call},
      call_remote1, call_remote2, calling_self,
      call_remote3, call_remote_n1, call_remote_n2,
@@ -1441,6 +1443,40 @@ handle_event_timeout_zero(Config) when is_list(Config) ->
     process_flag(trap_exit, OldFl),
     ok.
 
+handle_event_timeout_plain(Config) when is_list(Config) ->
+    OldFl = process_flag(trap_exit, true),
+
+    {ok, Pid} =
+        gen_server:start(gen_server_SUITE, [], []),
+    pong = gen_server:call(Pid, ping),
+
+    %% a `system` message should not cancel a plain timeout
+    ok = gen_server:cast(Pid, {self(), delayed_cast, 500}),
+    sys:get_status(Pid),
+    receive
+	{Pid, delayed} ->
+	    ok
+    after 1000 ->
+	ct:fail(delayed_cast_message_not_received)
+    end,
+
+    %% a request (or other message) should cancel a plain timeout
+    ok = gen_server:cast(Pid, {self(), delayed_cast, 500}),
+    pong = gen_server:call(Pid, ping),
+    receive
+	{Pid, delayed} ->
+	    ct:fail(delayed_cast_message_received)
+    after 1000 ->
+	ok
+    end,
+
+    ok = gen_server:call(Pid, stop),
+    busy_wait_for_process(Pid, 600),
+    {'EXIT', {noproc, _}} = (catch gen_server:call(Pid, started_p, 1)),
+
+    process_flag(trap_exit, OldFl),
+    ok.
+
 hibernate(Config) when is_list(Config) ->
     OldFl = process_flag(trap_exit, true),
     {ok, Pid0} =