[PATCH 13/14] signal.c: prevent spurious wakeup of main thread for SIGCHLD

[Eric Wong <e@80x24.org>]

This is necessary for test/-ext-/gvl/test_last_thread.rb to
pass with MJIT enabled, as that test fails if spurious wakeup
happens.
---
 signal.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/signal.c b/signal.c
index d09480fb6f..90d7824b2b 100644
--- a/signal.c
+++ b/signal.c
@@ -692,12 +692,29 @@ signal_enque(int sig)
     ATOMIC_INC(signal_buff.size);
 }
 
+static sig_atomic_t sigchld_hit;
+
+/* Prevent compiler from reordering access */
+#define ACCESS_ONCE(type,x) (*((volatile type *)&(x)))
+
 static RETSIGTYPE
 sighandler(int sig)
 {
     int old_errnum = errno;
 
-    signal_enque(sig);
+    /* the VM always needs to handle SIGCHLD for rb_waitpid */
+    if (sig == RUBY_SIGCHLD) {
+        rb_vm_t *vm = GET_VM();
+        sigchld_hit = 1;
+
+        /* avoid spurious wakeup in main thread iff nobody uses trap(:CHLD) */
+        if (vm && ACCESS_ONCE(VALUE, vm->trap_list.cmd[sig])) {
+            signal_enque(sig);
+        }
+    }
+    else {
+        signal_enque(sig);
+    }
     rb_thread_wakeup_timer_thread();
 #if !defined(BSD_SIGNAL) && !defined(POSIX_SIGNAL)
     ruby_signal(sig, sighandler);
@@ -1048,13 +1065,8 @@ void ruby_waitpid_all(rb_vm_t *); /* process.c */
 void
 ruby_sigchld_handler(rb_vm_t *vm)
 {
-    /*
-     * Checking signal_buff.cnt[RUBY_SIGCHLD] here is not completely
-     * reliable as it can race with rb_get_next_signal in the
-     * main thread.  However, this remains useful when the main thread
-     * is blocked in an uninterruptible state:
-     */
-    if (signal_buff.cnt[RUBY_SIGCHLD]) {
+    if (sigchld_hit) {
+        sigchld_hit = 0;
         ruby_waitpid_all(vm);
     }
 }
@@ -1066,14 +1078,6 @@ rb_signal_exec(rb_thread_t *th, int sig)
     VALUE cmd = vm->trap_list.cmd[sig];
     int safe = vm->trap_list.safe[sig];
 
-    /*
-     * This is necessary as rb_get_next_signal from this (main) thread
-     * can steal work from the timer-thread running ruby_sigchld_handler
-     */
-    if (RUBY_SIGCHLD == sig) {
-        ruby_waitpid_all(vm);
-    }
-
     if (cmd == 0) {
 	switch (sig) {
 	  case SIGINT:
@@ -1295,7 +1299,7 @@ trap(int sig, sighandler_t func, VALUE command)
 	break;
     }
 
-    vm->trap_list.cmd[sig] = command;
+    ACCESS_ONCE(VALUE, vm->trap_list.cmd[sig]) = command;
     vm->trap_list.safe[sig] = rb_safe_level();
 
     return oldcmd;
@@ -1450,7 +1454,7 @@ init_sigchld(int sig)
     oldfunc = ruby_signal(sig, SIG_DFL);
     if (oldfunc == SIG_ERR) return -1;
     ruby_signal(sig, func);
-    GET_VM()->trap_list.cmd[sig] = 0;
+    ACCESS_ONCE(VALUE, GET_VM()->trap_list.cmd[sig]) = 0;
 
     return 0;
 }
-- 
EW