normal	2018-08-18 18:07:36 +0900 (Sat, 18 Aug 2018)

  New Revision: 64444

  https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=64444

  Log:
    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.
    
    [ruby-core:88537] [Feature #15002]

  Modified files:
    trunk/signal.c
    trunk/thread.c
    trunk/vm_core.h
Index: vm_core.h
===================================================================
--- vm_core.h	(revision 64443)
+++ vm_core.h	(revision 64444)
@@ -1736,11 +1736,11 @@ enum { https://github.com/ruby/ruby/blob/trunk/vm_core.h#L1736
 
 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);
Index: signal.c
===================================================================
--- signal.c	(revision 64443)
+++ signal.c	(revision 64444)
@@ -1039,7 +1039,7 @@ sig_do_nothing(int sig) https://github.com/ruby/ruby/blob/trunk/signal.c#L1039
 }
 #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 https://github.com/ruby/ruby/blob/trunk/signal.c#L1053
      * 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 https://github.com/ruby/ruby/blob/trunk/signal.c#L1069
 	/* 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) https://github.com/ruby/ruby/blob/trunk/signal.c#L1094
     }
 }
 
-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) https://github.com/ruby/ruby/blob/trunk/signal.c#L1133
 	rb_threadptr_signal_exit(th);
     }
     else {
-	signal_exec(cmd, safe, sig);
+	return signal_exec(cmd, safe, sig);
     }
+    return FALSE;
 }
 
 static sighandler_t
Index: thread.c
===================================================================
--- thread.c	(revision 64443)
+++ thread.c	(revision 64444)
@@ -187,20 +187,24 @@ static inline void blocking_region_end(r https://github.com/ruby/ruby/blob/trunk/thread.c#L187
     }; \
 } 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 https://github.com/ruby/ruby/blob/trunk/thread.c#L1183
 {
     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 https://github.com/ruby/ruby/blob/trunk/thread.c#L1197
 	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 t https://github.com/ruby/ruby/blob/trunk/thread.c#L1288
 {
     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 t https://github.com/ruby/ruby/blob/trunk/thread.c#L1296
     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 https://github.com/ruby/ruby/blob/trunk/thread.c#L2159
     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_threa https://github.com/ruby/ruby/blob/trunk/thread.c#L2196
 	    }
 	    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_threa https://github.com/ruby/ruby/blob/trunk/thread.c#L2205
 	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_threa https://github.com/ruby/ruby/blob/trunk/thread.c#L2245
 	    rb_thread_schedule_limits(limits_us);
 	}
     }
+    return ret;
 }
 
 void

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/