From 0d83015f0fd569292b28c6879285805d2a1ed620 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 17 Aug 2018 08:00:40 +0000 Subject: [PATCH] thread.c (sleep_*): reduce the effect of spurious interrupts Spurious interrupts from SIGCHLD cause Mutex#sleep (via ConditionVariable#wait) to return early and breaks some use cases. Since these are outside the programs's control with MJIT, we will only consider pending interrupts (e.g. those from Thread#run) and signals which cause a Ruby-level Signal.trap handler to fire as "spurious" wakeups. --- signal.c | 11 +++++++---- thread.c | 29 +++++++++++++++++++---------- vm_core.h | 4 ++-- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/signal.c b/signal.c index 31dd3bbdb0..326179f383 100644 --- a/signal.c +++ b/signal.c @@ -1039,7 +1039,7 @@ sig_do_nothing(int sig) } #endif -static void +static int signal_exec(VALUE cmd, int safe, int sig) { rb_execution_context_t *ec = GET_EC(); @@ -1053,7 +1053,7 @@ signal_exec(VALUE cmd, int safe, int sig) * 3. rb_signal_exec runs on queued signal */ if (IMMEDIATE_P(cmd)) - return; + return FALSE; ec->interrupt_mask |= TRAP_INTERRUPT_MASK; EC_PUSH_TAG(ec); @@ -1069,6 +1069,7 @@ signal_exec(VALUE cmd, int safe, int sig) /* XXX: should be replaced with rb_threadptr_pending_interrupt_enque() */ EC_JUMP_TAG(ec, state); } + return TRUE; } void @@ -1093,7 +1094,8 @@ ruby_sigchld_handler(rb_vm_t *vm) } } -void +/* returns true if a trap handler was run, false otherwise */ +int rb_signal_exec(rb_thread_t *th, int sig) { rb_vm_t *vm = GET_VM(); @@ -1131,8 +1133,9 @@ rb_signal_exec(rb_thread_t *th, int sig) rb_threadptr_signal_exit(th); } else { - signal_exec(cmd, safe, sig); + return signal_exec(cmd, safe, sig); } + return FALSE; } static sighandler_t diff --git a/thread.c b/thread.c index 23a4ebf192..8254c7dc0c 100644 --- a/thread.c +++ b/thread.c @@ -187,20 +187,24 @@ static inline void blocking_region_end(rb_thread_t *th, struct rb_blocking_regio }; \ } while(0) +/* + * returns true if this thread was spuriously interrupted, false otherwise + * (e.g. hit by Thread#run or ran a Ruby-level Signal.trap handler) + */ #define RUBY_VM_CHECK_INTS_BLOCKING(ec) vm_check_ints_blocking(ec) -static inline void +static inline int vm_check_ints_blocking(rb_execution_context_t *ec) { rb_thread_t *th = rb_ec_thread_ptr(ec); if (LIKELY(rb_threadptr_pending_interrupt_empty_p(th))) { - if (LIKELY(!RUBY_VM_INTERRUPTED_ANY(ec))) return; + if (LIKELY(!RUBY_VM_INTERRUPTED_ANY(ec))) return FALSE; } else { th->pending_interrupt_queue_checked = 0; RUBY_VM_SET_INTERRUPT(ec); } - rb_threadptr_execute_interrupts(th, 1); + return rb_threadptr_execute_interrupts(th, 1); } static int @@ -1179,6 +1183,7 @@ sleep_forever(rb_thread_t *th, unsigned int fl) { enum rb_thread_status prev_status = th->status; enum rb_thread_status status; + int woke; status = fl & SLEEP_DEADLOCKABLE ? THREAD_STOPPED_FOREVER : THREAD_STOPPED; th->status = status; @@ -1192,8 +1197,8 @@ sleep_forever(rb_thread_t *th, unsigned int fl) if (fl & SLEEP_DEADLOCKABLE) { th->vm->sleeper--; } - RUBY_VM_CHECK_INTS_BLOCKING(th->ec); - if (!(fl & SLEEP_SPURIOUS_CHECK)) + woke = vm_check_ints_blocking(th->ec); + if (woke && !(fl & SLEEP_SPURIOUS_CHECK)) break; } th->status = prev_status; @@ -1283,6 +1288,7 @@ sleep_timespec(rb_thread_t *th, struct timespec ts, unsigned int fl) { struct timespec end; enum rb_thread_status prev_status = th->status; + int woke; getclockofday(&end); timespec_add(&end, &ts); @@ -1290,8 +1296,8 @@ sleep_timespec(rb_thread_t *th, struct timespec ts, unsigned int fl) RUBY_VM_CHECK_INTS_BLOCKING(th->ec); while (th->status == THREAD_STOPPED) { native_sleep(th, &ts); - RUBY_VM_CHECK_INTS_BLOCKING(th->ec); - if (!(fl & SLEEP_SPURIOUS_CHECK)) + woke = vm_check_ints_blocking(th->ec); + if (woke && !(fl & SLEEP_SPURIOUS_CHECK)) break; if (timespec_update_expire(&ts, &end)) break; @@ -2153,13 +2159,14 @@ threadptr_get_interrupts(rb_thread_t *th) return interrupt & (rb_atomic_t)~ec->interrupt_mask; } -MJIT_FUNC_EXPORTED void +MJIT_FUNC_EXPORTED int rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing) { rb_atomic_t interrupt; int postponed_job_interrupt = 0; + int ret = FALSE; - if (th->ec->raised_flag) return; + if (th->ec->raised_flag) return ret; while ((interrupt = threadptr_get_interrupts(th)) != 0) { int sig; @@ -2189,7 +2196,7 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing) } th->status = THREAD_RUNNABLE; while ((sig = rb_get_next_signal()) != 0) { - rb_signal_exec(th, sig); + ret |= rb_signal_exec(th, sig); } th->status = prev_status; } @@ -2198,6 +2205,7 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing) if (pending_interrupt && threadptr_pending_interrupt_active_p(th)) { VALUE err = rb_threadptr_pending_interrupt_deque(th, blocking_timing ? INTERRUPT_ON_BLOCKING : INTERRUPT_NONE); thread_debug("rb_thread_execute_interrupts: %"PRIdVALUE"\n", err); + ret = TRUE; if (err == Qundef) { /* no error */ @@ -2237,6 +2245,7 @@ rb_threadptr_execute_interrupts(rb_thread_t *th, int blocking_timing) rb_thread_schedule_limits(limits_us); } } + return ret; } void diff --git a/vm_core.h b/vm_core.h index 38a5775220..b936b0352b 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1736,11 +1736,11 @@ enum { VALUE rb_exc_set_backtrace(VALUE exc, VALUE bt); int rb_signal_buff_size(void); -void rb_signal_exec(rb_thread_t *th, int sig); +int rb_signal_exec(rb_thread_t *th, int sig); void rb_threadptr_check_signal(rb_thread_t *mth); void rb_threadptr_signal_raise(rb_thread_t *th, int sig); void rb_threadptr_signal_exit(rb_thread_t *th); -void rb_threadptr_execute_interrupts(rb_thread_t *, int); +int rb_threadptr_execute_interrupts(rb_thread_t *, int); void rb_threadptr_interrupt(rb_thread_t *th); void rb_threadptr_unlock_all_locking_mutexes(rb_thread_t *th); void rb_threadptr_pending_interrupt_clear(rb_thread_t *th); -- EW