Merge branch 'rickard/receive-bad-timeout-fix/24/OTP-17651' into maint-24

* rickard/receive-bad-timeout-fix/24/OTP-17651:
  Ensure valid state when recv mark is inserted into message queue
  Restore save pointer on bad timeout value in receive

5 files changed, 88 insertions, 5 deletions

diff --git a/erts/emulator/beam/emu/msg_instrs.tab b/erts/emulator/beam/emu/msg_instrs.tab
index 690f2f2af5..085cdb34cd 100644
--- a/erts/emulator/beam/emu/msg_instrs.tab
+++ b/erts/emulator/beam/emu/msg_instrs.tab
@@ -332,6 +332,7 @@ timeout() {
 
 TIMEOUT_VALUE() {
     c_p->freason = EXC_TIMEOUT_VALUE;
+    erts_msgq_set_save_first(c_p);
     goto find_func_info;
     //| -no_next
 }
@@ -399,6 +400,7 @@ wait.src(Src) {
             } else { /* Wrong time */
                 erts_proc_unlock(c_p, ERTS_PROC_LOCKS_MSG_RECEIVE);
                 c_p->freason = EXC_TIMEOUT_VALUE;
+                erts_msgq_set_save_first(c_p);
                 goto find_func_info;
             }
         }
diff --git a/erts/emulator/beam/erl_proc_sig_queue.c b/erts/emulator/beam/erl_proc_sig_queue.c
index feab6b6bc8..e638e499c9 100644
--- a/erts/emulator/beam/erl_proc_sig_queue.c
+++ b/erts/emulator/beam/erl_proc_sig_queue.c
@@ -3265,7 +3265,7 @@ recv_marker_alloc(Process *c_p, ErtsRecvMarkerBlock **blkpp,
 }
 
 static ERTS_INLINE void
-recv_marker_insert(Process *c_p, ErtsRecvMarker *markp)
+recv_marker_insert(Process *c_p, ErtsRecvMarker *markp, int setting)
 {
     ERTS_HDBG_CHECK_SIGNAL_PRIV_QUEUE(c_p, 0);
     markp->sig.common.next = NULL;
@@ -3283,6 +3283,17 @@ recv_marker_insert(Process *c_p, ErtsRecvMarker *markp)
         *c_p->sig_qs.last = (ErtsMessage *) &markp->sig;
         c_p->sig_qs.last = &markp->sig.common.next;
 
+        if (!setting && *c_p->sig_qs.save == (ErtsMessage *) &markp->sig) {
+            /*
+             * This should most likely never happen (which is why the assert
+             * is here), but if it does, leave the message queue in a valid
+             * state...
+             */
+            ASSERT(0);
+            markp->pass++;
+            c_p->sig_qs.save = c_p->sig_qs.last;
+        }
+        
 	ERTS_SIG_DBG_RECV_MARK_SET_HANDLED(&markp->sig);
     }
     else {
@@ -3319,7 +3330,7 @@ erts_msgq_recv_marker_create_insert(Process *c_p, Eterm uniq)
     ErtsRecvMarker *markp = recv_marker_alloc(c_p, blkpp, &ix, &new_uniq);
     if (!markp)
 	return am_undefined;
-    recv_marker_insert(c_p, markp);
+    recv_marker_insert(c_p, markp, 0);
     ASSERT(is_small(new_uniq) || is_big(new_uniq) || new_uniq == NIL
 	   || is_internal_ref(new_uniq));
     return new_uniq;
@@ -3333,7 +3344,7 @@ erts_msgq_recv_marker_create_insert_set_save(Process *c_p, Eterm id)
     ErtsRecvMarker *markp = recv_marker_alloc(c_p, blkpp, &ix, &id);
 
     if (markp) {
-	recv_marker_insert(c_p, markp);
+	recv_marker_insert(c_p, markp, !0);
 	erts_msgq_recv_marker_set_save__(c_p, *blkpp, markp, ix);
 	ASSERT(markp->in_sigq > 0);
 	ASSERT(!markp->in_msgq);
diff --git a/erts/emulator/beam/jit/beam_jit_common.c b/erts/emulator/beam/jit/beam_jit_common.c
index 9b56fb1ac1..a69a48beff 100644
--- a/erts/emulator/beam/jit/beam_jit_common.c
+++ b/erts/emulator/beam/jit/beam_jit_common.c
@@ -732,6 +732,7 @@ enum beam_jit_tmo_ret beam_jit_wait_timeout(Process *c_p,
             } else { /* Wrong time */
                 erts_proc_unlock(c_p, ERTS_PROC_LOCKS_MSG_RECEIVE);
                 c_p->freason = EXC_TIMEOUT_VALUE;
+                erts_msgq_set_save_first(c_p);
                 return RET_badarg;
             }
         }
diff --git a/erts/emulator/beam/jit/x86/instr_msg.cpp b/erts/emulator/beam/jit/x86/instr_msg.cpp
index e9110e174b..d0fa04d1ac 100644
--- a/erts/emulator/beam/jit/x86/instr_msg.cpp
+++ b/erts/emulator/beam/jit/x86/instr_msg.cpp
@@ -165,8 +165,17 @@ void BeamGlobalAssembler::emit_i_loop_rec_shared() {
     comment("Peek next message");
     a.bind(peek_message);
     {
+#ifdef DEBUG
+        /* Want asserts in erts_msgq_peek_msg()... */
+        emit_enter_runtime();
+        a.mov(ARG1, c_p);
+        runtime_call<1>(erts_msgq_peek_msg);
+        emit_leave_runtime();
+        a.mov(ARG1, RET);
+#else
         a.mov(ARG1, x86::qword_ptr(c_p, offsetof(Process, sig_qs.save)));
         a.mov(ARG1, x86::qword_ptr(ARG1));
+#endif
         a.test(ARG1, ARG1);
         a.jne(check_is_distributed);
 
diff --git a/erts/emulator/test/receive_SUITE.erl b/erts/emulator/test/receive_SUITE.erl
index 74d7fa6b91..748a6f0bcd 100644
--- a/erts/emulator/test/receive_SUITE.erl
+++ b/erts/emulator/test/receive_SUITE.erl
@@ -29,7 +29,9 @@
          receive_opt_exception/1,receive_opt_recursion/1,
          receive_opt_deferred_save/1,
          erl_1199/1, multi_recv_opt/1,
-	 multi_recv_opt_clear/1]).
+	 multi_recv_opt_clear/1,
+         gh_5235_missing_save_reset/1,
+         gh_5235_recv_mark/1]).
 
 suite() ->
     [{ct_hooks,[ts_install_cth]},
@@ -42,7 +44,9 @@ all() ->
      receive_opt_recursion,
      receive_opt_deferred_save,
      erl_1199, multi_recv_opt,
-     multi_recv_opt_clear].
+     multi_recv_opt_clear,
+     gh_5235_missing_save_reset,
+     gh_5235_recv_mark].
 
 init_per_testcase(receive_opt_deferred_save, Config) ->
     case erlang:system_info(schedulers_online) of
@@ -525,13 +529,69 @@ multi_call_clear(Srv) ->
 		   multi_call(Srv, 1, two9, 2, true, NoOp)
 	   end,
     multi_call(Srv, 1000, one, 100, true, Fun2).
+  
+gh_5235_missing_save_reset(Config) when is_list(Config) ->
+    %%
+    %% Used to hang in the second receive due to save
+    %% pointer not being reset on bad timeout value...
+    %%
+    ct:timetrap({seconds, 10}),
+    id(self()) ! init,
+    try
+        receive blipp -> ok after blupp -> ok end
+    catch _:_ ->
+            ok
+    end,
+    receive init -> ok end,
 
+    %% Try with a timeout value not known in compile
+    %% time as well...
+    id(self()) ! init2,
+    try
+        receive blapp -> ok after id(blepp) -> ok end
+    catch _:_ ->
+            ok
+    end,
+    receive init2 -> ok end.
+
+gh_5235_recv_mark(Config) when is_list(Config) ->
+    %%
+    %% Used to leak a recv marker into the receive
+    %% via erts_msgq_peek_msg() due to save pointer
+    %% not being reset on bad timeout value...
+    %%
+    ct:timetrap({seconds, 10}),
+    id(self()) ! init,
+    try
+        receive blipp -> ok after blupp -> ok end
+    catch _:_ ->
+            ok
+    end,
+    Ref = make_ref(),
+    id(self()) ! Ref,
+    receive init -> ok end,
+    receive Ref -> ok end,
     
+    %% Try with a timeout value not known in compile
+    %% time as well...
+    id(self()) ! init2,
+    try
+        receive blapp -> ok after id(blepp) -> ok end
+    catch _:_ ->
+            ok
+    end,
+    Ref2 = make_ref(),
+    id(self()) ! Ref2,
+    receive init2 -> ok end,
+    receive Ref2 -> ok end.
+
 
 %%%
 %%% Common helpers.
 %%%
 
+id(X) -> X.
+
 flush_msgq() ->
     flush_msgq(0).
 flush_msgq(N) ->