From 054a412d540e7ed2de63d68da753f585ea6616c3 Mon Sep 17 00:00:00 2001 From: normal Date: Wed, 27 Jun 2018 03:14:30 +0000 Subject: hijack SIGCHLD handler for internal use Use a global SIGCHLD handler to guard all callers of rb_waitpid. To work safely with multi-threaded programs, we introduce a VM-wide waitpid_lock to be acquired BEFORE fork/vfork spawns the process. This is to be combined with the new ruby_waitpid_locked function used by mjit.c in a non-Ruby thread. Ruby-level SIGCHLD handlers registered with Signal.trap(:CHLD) continues to work as before and there should be no regressions in any existing use cases. Splitting the wait queues for PID > 0 and groups (PID <= 0) ensures we favor PID > 0 callers. The disabling of SIGCHLD in rb_f_system is longer necessary, as we use deferred signal handling and no longer make ANY blocking waitpid syscalls in other threads which could "beat" the waitpid call made by rb_f_system. We prevent SIGCHLD from firing in normal Ruby Threads and only enable it in the timer-thread, to prevent spurious wakeups from in test/-ext-/gvl/test_last_thread.rb with MJIT enabled. I've tried to guard as much of the code for RUBY_SIGCHLD==0 using C "if" statements rather than CPP "#if" so to reduce the likelyhood of portability problems as the compiler will see more code. We also work to suppress false-positives from Process.wait(-1, Process::WNOHANG) to quiets warnings from spec/ruby/core/process/wait2_spec.rb with MJIT enabled. Lastly, we must implement rb_grantpt for ext/pty. We need a MJIT-compatible way of supporting grantpt(3) which may spawn the `pt_chown' binary and call waitpid(2) on it. [ruby-core:87605] [Ruby trunk Bug#14867] git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@63758 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- signal.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 100 insertions(+), 26 deletions(-) (limited to 'signal.c') diff --git a/signal.c b/signal.c index c781c38c62..7d2226b766 100644 --- a/signal.c +++ b/signal.c @@ -62,10 +62,6 @@ ruby_atomic_compare_and_swap(rb_atomic_t *ptr, rb_atomic_t cmp, } #endif -#ifndef NSIG -# define NSIG (_SIGMAX + 1) /* For QNX */ -#endif - static const struct signals { const char *signm; int signo; @@ -129,15 +125,9 @@ static const struct signals { #ifdef SIGCONT {"CONT", SIGCONT}, #endif -#ifdef SIGCHLD - {"CHLD", SIGCHLD}, -#endif -#ifdef SIGCLD - {"CLD", SIGCLD}, -#else -# ifdef SIGCHLD - {"CLD", SIGCHLD}, -# endif +#if RUBY_SIGCHLD + {"CHLD", RUBY_SIGCHLD }, + {"CLD", RUBY_SIGCHLD }, #endif #ifdef SIGTTIN {"TTIN", SIGTTIN}, @@ -702,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); @@ -742,6 +749,7 @@ rb_enable_interrupt(void) #ifdef HAVE_PTHREAD_SIGMASK sigset_t mask; sigemptyset(&mask); + sigaddset(&mask, RUBY_SIGCHLD); /* timer-thread handles this */ pthread_sigmask(SIG_SETMASK, &mask, NULL); #endif } @@ -1052,6 +1060,18 @@ rb_trap_exit(void) } } +void ruby_waitpid_all(rb_vm_t *); /* process.c */ + +/* only runs in the timer-thread */ +void +ruby_sigchld_handler(rb_vm_t *vm) +{ + if (sigchld_hit) { + sigchld_hit = 0; + ruby_waitpid_all(vm); + } +} + void rb_signal_exec(rb_thread_t *th, int sig) { @@ -1117,6 +1137,9 @@ default_handler(int sig) #endif #ifdef SIGUSR2 case SIGUSR2: +#endif +#if RUBY_SIGCHLD + case RUBY_SIGCHLD: #endif func = sighandler; break; @@ -1155,6 +1178,9 @@ trap_handler(VALUE *cmd, int sig) VALUE command; if (NIL_P(*cmd)) { + if (sig == RUBY_SIGCHLD) { + goto sig_dfl; + } func = SIG_IGN; } else { @@ -1175,6 +1201,9 @@ trap_handler(VALUE *cmd, int sig) break; case 14: if (memcmp(cptr, "SYSTEM_DEFAULT", 14) == 0) { + if (sig == RUBY_SIGCHLD) { + goto sig_dfl; + } func = SIG_DFL; *cmd = 0; } @@ -1182,6 +1211,9 @@ trap_handler(VALUE *cmd, int sig) case 7: if (memcmp(cptr, "SIG_IGN", 7) == 0) { sig_ign: + if (sig == RUBY_SIGCHLD) { + goto sig_dfl; + } func = SIG_IGN; *cmd = Qtrue; } @@ -1268,7 +1300,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; @@ -1413,20 +1445,18 @@ install_sighandler(int signum, sighandler_t handler) # define install_sighandler(signum, handler) \ INSTALL_SIGHANDLER(install_sighandler(signum, handler), #signum, signum) -#if defined(SIGCLD) || defined(SIGCHLD) +#if RUBY_SIGCHLD static int init_sigchld(int sig) { sighandler_t oldfunc; + sighandler_t func = sighandler; oldfunc = ruby_signal(sig, SIG_DFL); if (oldfunc == SIG_ERR) return -1; - if (oldfunc != SIG_DFL && oldfunc != SIG_IGN) { - ruby_signal(sig, oldfunc); - } - else { - GET_VM()->trap_list.cmd[sig] = 0; - } + ruby_signal(sig, func); + ACCESS_ONCE(VALUE, GET_VM()->trap_list.cmd[sig]) = 0; + return 0; } @@ -1542,11 +1572,55 @@ Init_signal(void) install_sighandler(SIGSYS, sig_do_nothing); #endif -#if defined(SIGCLD) - init_sigchld(SIGCLD); -#elif defined(SIGCHLD) - init_sigchld(SIGCHLD); +#if RUBY_SIGCHLD + init_sigchld(RUBY_SIGCHLD); #endif rb_enable_interrupt(); } + +#if defined(HAVE_GRANTPT) +extern int grantpt(int); +#else +static int +fake_grantfd(int masterfd) +{ + errno = ENOSYS; + return -1; +} +#define grantpt(fd) fake_grantfd(int) +#endif + +int +rb_grantpt(int masterfd) +{ + if (RUBY_SIGCHLD) { + rb_vm_t *vm = GET_VM(); + int ret, e; + + /* + * Prevent waitpid calls from Ruby by taking waitpid_lock. + * Pedantically, grantpt(3) is undefined if a non-default + * SIGCHLD handler is defined, but preventing conflicting + * waitpid calls ought to be sufficient. + * + * We could install the default sighandler temporarily, but that + * could cause SIGCHLD to be missed by other threads. Blocking + * SIGCHLD won't work here, either, unless we stop and restart + * timer-thread (as only timer-thread sees SIGCHLD), but that + * seems like overkill. + */ + rb_nativethread_lock_lock(&vm->waitpid_lock); + { + ret = grantpt(masterfd); /* may spawn `pt_chown' and wait on it */ + if (ret < 0) e = errno; + } + rb_nativethread_lock_unlock(&vm->waitpid_lock); + + if (ret < 0) errno = e; + return ret; + } + else { + return grantpt(masterfd); + } +} -- cgit v1.2.3