diff options
author | Rickard Green <rickard@erlang.org> | 2023-01-17 17:51:48 +0100 |
---|---|---|
committer | Rickard Green <rickard@erlang.org> | 2023-01-17 17:51:48 +0100 |
commit | 50203fc59c2d6e509fa6333e39c258de433c04fc (patch) | |
tree | 9bbd3f759b7c54385d7986fdd9fdd6c7e4c28360 /erts/emulator/beam | |
parent | cb658b93c34209ccd2234d647f05af44bc27c5a8 (diff) | |
parent | 8da67f71b885916a382bd1ea43f7a02c5f1d6b4b (diff) | |
download | erlang-50203fc59c2d6e509fa6333e39c258de433c04fc.tar.gz |
Merge branch 'rickard/signal-handling-fix/25.2/OTP-18388' into maint
* rickard/signal-handling-fix/25.2/OTP-18388:
[erts] Better testcase for dirty signal handling race
[erts] Prevent execution of a process while dirty signal handling is done
Diffstat (limited to 'erts/emulator/beam')
-rw-r--r-- | erts/emulator/beam/erl_bif_info.c | 57 | ||||
-rw-r--r-- | erts/emulator/beam/erl_proc_sig_queue.c | 45 | ||||
-rw-r--r-- | erts/emulator/beam/erl_proc_sig_queue.h | 50 | ||||
-rw-r--r-- | erts/emulator/beam/erl_process.c | 7 |
4 files changed, 137 insertions, 22 deletions
diff --git a/erts/emulator/beam/erl_bif_info.c b/erts/emulator/beam/erl_bif_info.c index 17e194847c..c98e5777eb 100644 --- a/erts/emulator/beam/erl_bif_info.c +++ b/erts/emulator/beam/erl_bif_info.c @@ -4667,6 +4667,44 @@ test_multizero_timeout_in_timeout(void *vproc) erts_start_timer_callback(0, test_multizero_timeout_in_timeout2, vproc); } +static Eterm +proc_sig_block(Process *c_p, void *arg, int *redsp, ErlHeapFragment **bpp) +{ + ErtsMonotonicTime time, timeout_time, ms = (ErtsMonotonicTime) (Sint) arg; + + if (redsp) + *redsp = 1; + + time = erts_get_monotonic_time(NULL); + + if (ms < 0) + timeout_time = time; + else + timeout_time = time + ERTS_MSEC_TO_MONOTONIC(ms); + + while (time < timeout_time) { + ErtsMonotonicTime timeout = timeout_time - time; + +#ifdef __WIN32__ + Sleep((DWORD) ERTS_MONOTONIC_TO_MSEC(timeout)); +#else + { + ErtsMonotonicTime to = ERTS_MONOTONIC_TO_USEC(timeout); + struct timeval tv; + + tv.tv_sec = (long) to / (1000*1000); + tv.tv_usec = (long) to % (1000*1000); + + select(0, NULL, NULL, NULL, &tv); + } +#endif + + time = erts_get_monotonic_time(NULL); + } + + return am_ok; +} + BIF_RETTYPE erts_debug_set_internal_state_2(BIF_ALIST_2) { /* @@ -5071,6 +5109,25 @@ BIF_RETTYPE erts_debug_set_internal_state_2(BIF_ALIST_2) BIF_RET(am_ok); } } + else if (ERTS_IS_ATOM_STR("proc_sig_block", BIF_ARG_1)) { + if (is_tuple_arity(BIF_ARG_2, 2)) { + Eterm *tp = tuple_val(BIF_ARG_2); + Sint64 time; + if (is_internal_pid(tp[1]) && term_to_Sint64(tp[2], &time)) { + ErtsMonotonicTime wait_time = time; + Eterm res; + + res = erts_proc_sig_send_rpc_request(BIF_P, + tp[1], + 0, + proc_sig_block, + (void *) (Sint) wait_time); + if (is_non_value(res)) + BIF_RET(am_false); + BIF_RET(am_true); + } + } + } } BIF_ERROR(BIF_P, BADARG); diff --git a/erts/emulator/beam/erl_proc_sig_queue.c b/erts/emulator/beam/erl_proc_sig_queue.c index 1d80ac6793..a08f952d10 100644 --- a/erts/emulator/beam/erl_proc_sig_queue.c +++ b/erts/emulator/beam/erl_proc_sig_queue.c @@ -235,7 +235,6 @@ typedef struct { ErtsMessage **last; } ErtsSavedNMSignals; -static erts_aint32_t wait_handle_signals(Process *c_p); static void wake_handle_signals(Process *proc); static int handle_msg_tracing(Process *c_p, @@ -5423,12 +5422,8 @@ erts_proc_sig_handle_incoming(Process *c_p, erts_aint32_t *statep, ErtsMessage *sig, ***next_nm_sig; ErtsSigRecvTracing tracing; ErtsSavedNMSignals delayed_nm_signals = {0}; - - ASSERT(!(c_p->sig_qs.flags & FS_WAIT_HANDLE_SIGS)); - if (c_p->sig_qs.flags & FS_HANDLING_SIGS) - state = wait_handle_signals(c_p); - else - c_p->sig_qs.flags |= FS_HANDLING_SIGS; + + ASSERT(!(c_p->sig_qs.flags & (FS_WAIT_HANDLE_SIGS|FS_HANDLING_SIGS))); ERTS_HDBG_CHECK_SIGNAL_PRIV_QUEUE(c_p, 0); ERTS_LC_ASSERT(ERTS_PROC_LOCK_MAIN == erts_proc_lc_my_proc_locks(c_p)); @@ -5441,6 +5436,8 @@ erts_proc_sig_handle_incoming(Process *c_p, erts_aint32_t *statep, } } + c_p->sig_qs.flags |= FS_HANDLING_SIGS; + limit = *redsp; *redsp = 0; yield = 0; @@ -8050,14 +8047,16 @@ erts_proc_sig_cleanup_queues(Process *c_p) #endif } -/* Debug */ - -static erts_aint32_t -wait_handle_signals(Process *c_p) +void +erts_proc_sig_do_wait_dirty_handle_signals__(Process *c_p) { /* - * Process needs to wait on a dirty process signal - * handler before it can handle signals by itself... + * A dirty process signal handler is currently handling + * signals for this process, so it is not safe for this + * process to continue to execute. This process needs to + * wait for the dirty signal handling to complete before + * it can continue executing. This since otherwise the + * signal queue can be seen in an inconsistent state. * * This should be a quite rare event. This only occurs * when all of the following occurs: @@ -8066,14 +8065,17 @@ wait_handle_signals(Process *c_p) * * A dirty process signal handler starts handling * signals for the process and unlocks the main * lock while doing so. This can currently only - * occur if handling an 'unlink' signal from a port. + * occur if handling an 'unlink' signal from a port, or + * when handling an alias message where the alias + * has been created when monitoring a port using + * '{alias, reply_demonitor}' option. * * While the dirty process signal handler is handling * signals for the process, the process stops executing * dirty, gets scheduled on a normal scheduler, and * then tries to handle signals itself. * - * If the above happens, the normal sceduler executing - * the process will wait here until the dirty process + * If the above happens, the normal scheduler scheduling + * in the process will wait here until the dirty process * signal handler is done with the process... */ erts_tse_t *event; @@ -8101,20 +8103,17 @@ wait_handle_signals(Process *c_p) erts_tse_return(event); c_p->sig_qs.flags &= ~FS_WAIT_HANDLE_SIGS; - c_p->sig_qs.flags |= FS_HANDLING_SIGS; - - return erts_atomic32_read_mb(&c_p->state); } static void wake_handle_signals(Process *proc) { /* - * Wake scheduler sleeping in wait_handle_signals() + * Wake scheduler waiting in erts_proc_sig_check_wait_dirty_handle_signals() * (above)... * - * This function should only be called by a dirty process - * signal handler process... + * This function should only be called by a dirty process signal handler + * process... */ #ifdef DEBUG Process *c_p = erts_get_current_process(); @@ -8132,6 +8131,8 @@ wake_handle_signals(Process *proc) erts_tse_set(proc->scheduler_data->aux_work_data.ssi->event); } +/* Debug */ + static void debug_foreach_sig_heap_frags(ErlHeapFragment *hfrag, void (*oh_func)(ErlOffHeap *, void *), diff --git a/erts/emulator/beam/erl_proc_sig_queue.h b/erts/emulator/beam/erl_proc_sig_queue.h index 8faa79507f..b99e0befc9 100644 --- a/erts/emulator/beam/erl_proc_sig_queue.h +++ b/erts/emulator/beam/erl_proc_sig_queue.h @@ -1513,6 +1513,26 @@ int erts_proc_sig_decode_dist(Process *proc, ErtsProcLocks proc_locks, ErtsMessage *msgp, int force_off_heap); +/** + * + * @brief Check if a newly scheduled process needs to wait for + * ongoing dirty handling of signals to complete. + * + * @param[in] c_p Pointer to executing process. + * + * @param[in] state_in State of process. + * + * @return Updated state of process if + * we had to wait; otherwise, + * state_in. + */ +ERTS_GLB_INLINE erts_aint32_t +erts_proc_sig_check_wait_dirty_handle_signals(Process *c_p, + erts_aint32_t state_in); + +void erts_proc_sig_do_wait_dirty_handle_signals__(Process *c_p); + + ErtsDistExternal * erts_proc_sig_get_external(ErtsMessage *msgp); @@ -1789,6 +1809,8 @@ erts_proc_sig_fetch(Process *proc) || ERTS_PROC_IS_EXITING(proc) || ERTS_IS_CRASH_DUMPING); + ASSERT(!(proc->sig_qs.flags & FS_HANDLING_SIGS)); + ERTS_HDBG_CHECK_SIGNAL_IN_QUEUE(proc); ERTS_HDBG_CHECK_SIGNAL_PRIV_QUEUE(proc, !0); @@ -1965,6 +1987,7 @@ erts_msgq_recv_marker_clear(Process *c_p, Eterm id) { ErtsRecvMarkerBlock *blkp = c_p->sig_qs.recv_mrk_blk; int ix; + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); if (!is_small(id) && !is_big(id) && !is_internal_ref(id)) return; @@ -1992,6 +2015,7 @@ erts_msgq_recv_marker_clear(Process *c_p, Eterm id) ERTS_GLB_INLINE Eterm erts_msgq_recv_marker_insert(Process *c_p) { + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); erts_proc_sig_queue_lock(c_p); erts_proc_sig_fetch(c_p); erts_proc_unlock(c_p, ERTS_PROC_LOCK_MSGQ); @@ -2005,6 +2029,7 @@ ERTS_GLB_INLINE void erts_msgq_recv_marker_bind(Process *c_p, Eterm insert_id, Eterm bind_id) { + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); #ifdef ERTS_SUPPORT_OLD_RECV_MARK_INSTRS ASSERT(bind_id != erts_old_recv_marker_id); #endif @@ -2034,6 +2059,7 @@ ERTS_GLB_INLINE void erts_msgq_recv_marker_bind(Process *c_p, ERTS_GLB_INLINE void erts_msgq_recv_marker_insert_bind(Process *c_p, Eterm id) { + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); if (is_internal_ref(id)) { #ifdef ERTS_SUPPORT_OLD_RECV_MARK_INSTRS ErtsRecvMarkerBlock *blkp = c_p->sig_qs.recv_mrk_blk; @@ -2053,6 +2079,7 @@ erts_msgq_recv_marker_insert_bind(Process *c_p, Eterm id) ERTS_GLB_INLINE void erts_msgq_recv_marker_set_save(Process *c_p, Eterm id) { + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); if (is_internal_ref(id)) { ErtsRecvMarkerBlock *blkp = c_p->sig_qs.recv_mrk_blk; @@ -2073,6 +2100,7 @@ erts_msgq_recv_marker_set_save(Process *c_p, Eterm id) ERTS_GLB_INLINE ErtsMessage * erts_msgq_peek_msg(Process *c_p) { + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); ASSERT(!(*c_p->sig_qs.save) || ERTS_SIG_IS_MSG(*c_p->sig_qs.save)); return *c_p->sig_qs.save; } @@ -2081,6 +2109,7 @@ ERTS_GLB_INLINE void erts_msgq_unlink_msg(Process *c_p, ErtsMessage *msgp) { ErtsMessage *sigp = msgp->next; + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); ERTS_HDBG_CHECK_SIGNAL_PRIV_QUEUE__(c_p, 0, "before"); *c_p->sig_qs.save = sigp; c_p->sig_qs.len--; @@ -2099,6 +2128,7 @@ ERTS_GLB_INLINE void erts_msgq_set_save_first(Process *c_p) { ErtsRecvMarkerBlock *blkp = c_p->sig_qs.recv_mrk_blk; + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); if (blkp) { ERTS_PROC_SIG_RECV_MARK_CLEAR_PENDING_SET_SAVE__(blkp); #ifdef ERTS_SUPPORT_OLD_RECV_MARK_INSTRS @@ -2121,6 +2151,7 @@ ERTS_GLB_INLINE void erts_msgq_unlink_msg_set_save_first(Process *c_p, ErtsMessage *msgp) { ErtsMessage *sigp = msgp->next; + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); ERTS_HDBG_CHECK_SIGNAL_PRIV_QUEUE__(c_p, 0, "before"); *c_p->sig_qs.save = sigp; c_p->sig_qs.len--; @@ -2137,6 +2168,7 @@ erts_msgq_set_save_next(Process *c_p) { ErtsMessage *sigp = (*c_p->sig_qs.save)->next; ErtsMessage **sigpp = &(*c_p->sig_qs.save)->next; + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); ERTS_HDBG_CHECK_SIGNAL_PRIV_QUEUE(c_p, 0); if (sigp && ERTS_SIG_IS_RECV_MARKER(sigp)) sigpp = erts_msgq_pass_recv_markers(c_p, sigpp); @@ -2148,6 +2180,7 @@ ERTS_GLB_INLINE void erts_msgq_set_save_end(Process *c_p) { /* Set save pointer to end of message queue... */ + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); erts_proc_sig_queue_lock(c_p); erts_proc_sig_fetch(c_p); @@ -2167,6 +2200,23 @@ erts_msgq_set_save_end(Process *c_p) #undef ERTS_PROC_SIG_RECV_MARK_CLEAR_PENDING_SET_SAVE__ #undef ERTS_PROC_SIG_RECV_MARK_CLEAR_OLD_MARK__ +ERTS_GLB_INLINE erts_aint32_t +erts_proc_sig_check_wait_dirty_handle_signals(Process *c_p, + erts_aint32_t state_in) +{ + erts_aint32_t state = state_in; + ASSERT(erts_get_scheduler_data()->type == ERTS_SCHED_NORMAL); + ERTS_LC_ASSERT(ERTS_PROC_LOCK_MAIN == erts_proc_lc_my_proc_locks(c_p)); + + if (c_p->sig_qs.flags & FS_HANDLING_SIGS) { + erts_proc_sig_do_wait_dirty_handle_signals__(c_p); + state = erts_atomic32_read_mb(&c_p->state); + } + ERTS_LC_ASSERT(ERTS_PROC_LOCK_MAIN == erts_proc_lc_my_proc_locks(c_p)); + ASSERT(!(c_p->sig_qs.flags & FS_HANDLING_SIGS)); + return state; +} + #endif /* ERTS_GLB_INLINE_INCL_FUNC_DEF */ #endif /* ERTS_PROC_SIG_QUEUE_H__ */ diff --git a/erts/emulator/beam/erl_process.c b/erts/emulator/beam/erl_process.c index 3e56d6c62b..94c5745797 100644 --- a/erts/emulator/beam/erl_process.c +++ b/erts/emulator/beam/erl_process.c @@ -10083,6 +10083,13 @@ Process *erts_schedule(ErtsSchedulerData *esdp, Process *p, int calls) } else { /* On normal scheduler */ + + /* + * Check if a dirty signal handler is handling signals for + * us and if so, wait for it to complete before continuing... + */ + state = erts_proc_sig_check_wait_dirty_handle_signals(p, state); + if (state & ERTS_PSFLG_RUNNING_SYS) { if (state & (ERTS_PSFLG_SIG_Q|ERTS_PSFLG_SIG_IN_Q)) { int sig_reds; |