From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-3.9 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.1 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id 52DDA1F90A for ; Tue, 26 Jun 2018 09:38:20 +0000 (UTC) From: Eric Wong To: spew@80x24.org Subject: [PATCH 13/14] signal.c: prevent spurious wakeup of main thread for SIGCHLD Date: Tue, 26 Jun 2018 09:38:16 +0000 Message-Id: <20180626093817.1533-14-e@80x24.org> In-Reply-To: <20180626093817.1533-1-e@80x24.org> References: <20180626093817.1533-1-e@80x24.org> List-Id: 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