File 0300-Ensure-valid-state-when-recv-mark-is-inser... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 0300-Ensure-valid-state-when-recv-mark-is-inserted-into-m.patch of Package erlang (Revision f3783aa87a49d83f9834ec95bdd96e06)

Currently displaying revision f3783aa87a49d83f9834ec95bdd96e06 , Show latest

From b87c426ac0033bd01925782f1ea1253cfe17b54a Mon Sep 17 00:00:00 2001
From: Rickard Green <rickard@erlang.org>
Date: Thu, 23 Sep 2021 22:32:09 +0200
Subject: [PATCH] Ensure valid state when recv mark is inserted into message
 queue

---
 erts/emulator/beam/erl_proc_sig_queue.c  | 17 +++++++++--
 erts/emulator/beam/jit/x86/instr_msg.cpp |  9 ++++++
 erts/emulator/test/receive_SUITE.erl     | 38 ++++++++++++++++++++++--
 3 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/erts/emulator/beam/erl_proc_sig_queue.c b/erts/emulator/beam/erl_proc_sig_queue.c
index d31813aaa7..49c03739c3 100644
--- a/erts/emulator/beam/erl_proc_sig_queue.c
+++ b/erts/emulator/beam/erl_proc_sig_queue.c
@@ -3251,7 +3251,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;
@@ -3269,6 +3269,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 {
@@ -3305,7 +3316,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;
@@ -3319,7 +3330,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/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 5b9d9b7ce9..748a6f0bcd 100644
--- a/erts/emulator/test/receive_SUITE.erl
+++ b/erts/emulator/test/receive_SUITE.erl
          receive_opt_deferred_save/1,
          erl_1199/1, multi_recv_opt/1,
 	 multi_recv_opt_clear/1,
-         gh_5235_missing_save_reset/1]).
+         gh_5235_missing_save_reset/1,
+         gh_5235_recv_mark/1]).
 
 suite() ->
     [{ct_hooks,[ts_install_cth]},
@@ -44,7 +45,8 @@ all() ->
      receive_opt_deferred_save,
      erl_1199, multi_recv_opt,
      multi_recv_opt_clear,
-     gh_5235_missing_save_reset].
+     gh_5235_missing_save_reset,
+     gh_5235_recv_mark].
 
 init_per_testcase(receive_opt_deferred_save, Config) ->
     case erlang:system_info(schedulers_online) of
@@ -552,6 +554,38 @@ gh_5235_missing_save_reset(Config) when is_list(Config) ->
 
     
 
+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.
 %%%
-- 
2.31.1

Places

File 0300-Ensure-valid-state-when-recv-mark-is-inserted-into-m.patch of Package erlang (Revision f3783aa87a49d83f9834ec95bdd96e06)

Places