From 2be04f53570066031c4fc152aa14798dc38c0403 Mon Sep 17 00:00:00 2001 From: Rickard Green Date: Mon, 27 Sep 2021 14:09:04 +0200 Subject: Restore save pointer on bad timeout value in receive The message queue save pointer was not restored to the beginning of the message queue when a bad timeout value was encountered in the receive. If the timeout_value exception was caught, the following receive would not be searched from the beginning of the message queue. This potentially caused the process to hang forever even though a matching message was present in the message queue. --- erts/emulator/beam/msg_instrs.tab | 2 ++ erts/emulator/test/receive_SUITE.erl | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/erts/emulator/beam/msg_instrs.tab b/erts/emulator/beam/msg_instrs.tab index 10fd5ad48d..7056bbc95b 100644 --- a/erts/emulator/beam/msg_instrs.tab +++ b/erts/emulator/beam/msg_instrs.tab @@ -305,6 +305,7 @@ timeout() { TIMEOUT_VALUE() { c_p->freason = EXC_TIMEOUT_VALUE; + JOIN_MESSAGE(c_p); goto find_func_info; //| -no_next } @@ -372,6 +373,7 @@ wait.src(Src) { } else { /* Wrong time */ erts_proc_unlock(c_p, ERTS_PROC_LOCKS_MSG_RECEIVE); c_p->freason = EXC_TIMEOUT_VALUE; + JOIN_MESSAGE(c_p); goto find_func_info; } } diff --git a/erts/emulator/test/receive_SUITE.erl b/erts/emulator/test/receive_SUITE.erl index 339507c9d8..d42db517c2 100644 --- a/erts/emulator/test/receive_SUITE.erl +++ b/erts/emulator/test/receive_SUITE.erl @@ -28,7 +28,8 @@ call_with_huge_message_queue/1,receive_in_between/1, receive_opt_exception/1,receive_opt_recursion/1, receive_opt_deferred_save/1, - erl_1199/1]). + erl_1199/1, + gh_5235_missing_save_reset/1]). suite() -> [{ct_hooks,[ts_install_cth]}, @@ -40,7 +41,8 @@ all() -> receive_opt_exception, receive_opt_recursion, receive_opt_deferred_save, - erl_1199]. + erl_1199, + gh_5235_missing_save_reset]. init_per_testcase(receive_opt_deferred_save, Config) -> case erlang:system_info(schedulers) of @@ -396,10 +398,36 @@ erl_1199_flush_blipp() -> ok end. +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. + %%% %%% Common helpers. %%% +id(X) -> X. + echo_loop() -> receive {Ref,{Pid,Msg}} -> -- cgit v1.2.1 From b87c426ac0033bd01925782f1ea1253cfe17b54a Mon Sep 17 00:00:00 2001 From: Rickard Green Date: Thu, 23 Sep 2021 22:32:09 +0200 Subject: 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 @@ -30,7 +30,8 @@ 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) -> 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. %%% -- cgit v1.2.1