ruby-changes:51667
From: normal <ko1@a...>
Date: Sun, 8 Jul 2018 09:02:32 +0900 (JST)
Subject: [ruby-changes:51667] normal:r63879 (trunk): signal.c: preserve trap(:CHLD, "IGNORE") behavior with SIGCHLD
normal 2018-07-08 09:02:27 +0900 (Sun, 08 Jul 2018) New Revision: 63879 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=63879 Log: signal.c: preserve trap(:CHLD, "IGNORE") behavior with SIGCHLD We need to preserve "IGNORE" behavior from Ruby 2.5 and earlier. We can't rely on SA_NOCLDWAIT any more, since we always need system() and MJIT to work; so we fake that behavior using dedicated reaper (currently in timer-thread). Modified files: trunk/internal.h trunk/process.c trunk/signal.c trunk/test/ruby/test_signal.rb Index: signal.c =================================================================== --- signal.c (revision 63878) +++ signal.c (revision 63879) @@ -531,6 +531,7 @@ static struct { https://github.com/ruby/ruby/blob/trunk/signal.c#L531 rb_atomic_t cnt[RUBY_NSIG]; rb_atomic_t size; } signal_buff; +volatile unsigned int ruby_nocldwait; #ifdef __dietlibc__ #define sighandler_t sh_t @@ -614,12 +615,20 @@ ruby_signal(int signum, sighandler_t han https://github.com/ruby/ruby/blob/trunk/signal.c#L615 #endif switch (signum) { -#ifdef SA_NOCLDWAIT case SIGCHLD: - if (handler == SIG_IGN) - sigact.sa_flags |= SA_NOCLDWAIT; + if (handler == SIG_IGN) { + ruby_nocldwait = 1; + if (sigact.sa_flags & SA_SIGINFO) { + sigact.sa_sigaction = (ruby_sigaction_t*)sighandler; + } + else { + sigact.sa_handler = sighandler; + } + } + else { + ruby_nocldwait = 0; + } break; -#endif #if defined(SA_ONSTACK) && defined(USE_SIGALTSTACK) case SIGSEGV: #ifdef SIGBUS @@ -1183,9 +1192,6 @@ trap_handler(VALUE *cmd, int sig) https://github.com/ruby/ruby/blob/trunk/signal.c#L1192 VALUE command; if (NIL_P(*cmd)) { - if (sig == RUBY_SIGCHLD) { - goto sig_dfl; - } func = SIG_IGN; } else { @@ -1216,9 +1222,6 @@ trap_handler(VALUE *cmd, int sig) https://github.com/ruby/ruby/blob/trunk/signal.c#L1222 case 7: if (memcmp(cptr, "SIG_IGN", 7) == 0) { sig_ign: - if (sig == RUBY_SIGCHLD) { - goto sig_dfl; - } func = SIG_IGN; *cmd = Qtrue; } Index: internal.h =================================================================== --- internal.h (revision 63878) +++ internal.h (revision 63879) @@ -1683,6 +1683,7 @@ struct rb_execarg { https://github.com/ruby/ruby/blob/trunk/internal.h#L1683 unsigned uid_given : 1; unsigned gid_given : 1; unsigned exception : 1; + unsigned nocldwait_prev : 1; rb_pid_t pgroup_pgid; /* asis(-1), new pgroup(0), specified pgroup (0<V). */ VALUE rlimit_limits; /* Qfalse or [[rtype, softlim, hardlim], ...] */ mode_t umask_mask; Index: process.c =================================================================== --- process.c (revision 63878) +++ process.c (revision 63879) @@ -935,6 +935,7 @@ waitpid_notify(struct waitpid_state *w, https://github.com/ruby/ruby/blob/trunk/process.c#L935 # define waitpid_sys(pid,status,options) do_waitpid((pid),(status),(options)) #endif +extern volatile unsigned int ruby_nocldwait; /* signal.c */ /* called by timer thread */ static void waitpid_each(struct list_head *head) @@ -973,6 +974,11 @@ ruby_waitpid_all(rb_vm_t *vm) https://github.com/ruby/ruby/blob/trunk/process.c#L974 if (list_empty(&vm->waiting_pids)) { waitpid_each(&vm->waiting_grps); } + /* emulate SA_NOCLDWAIT */ + if (list_empty(&vm->waiting_pids) && list_empty(&vm->waiting_grps)) { + while (ruby_nocldwait && do_waitpid(-1, 0, WNOHANG) > 0) + ; /* keep looping */ + } rb_native_mutex_unlock(&vm->waitpid_lock); } @@ -1153,10 +1159,18 @@ rb_waitpid(rb_pid_t pid, int *st, int fl https://github.com/ruby/ruby/blob/trunk/process.c#L1159 } if (st) *st = w.status; - if (w.ret > 0) { - rb_last_status_set(w.status, w.ret); + if (w.ret == -1) { + errno = w.errnum; + } + else if (w.ret > 0) { + if (ruby_nocldwait) { + w.ret = -1; + errno = ECHILD; + } + else { + rb_last_status_set(w.status, w.ret); + } } - if (w.ret == -1) errno = w.errnum; return w.ret; } @@ -4306,16 +4320,22 @@ rb_f_system(int argc, VALUE *argv) https://github.com/ruby/ruby/blob/trunk/process.c#L4320 struct rb_execarg *eargp; execarg_obj = rb_execarg_new(argc, argv, TRUE, TRUE); + TypedData_Get_Struct(execarg_obj, struct rb_execarg, &exec_arg_data_type, eargp); + eargp->nocldwait_prev = ruby_nocldwait; + ruby_nocldwait = 0; pid = rb_execarg_spawn(execarg_obj, NULL, 0); #if defined(HAVE_WORKING_FORK) || defined(HAVE_SPAWNV) if (pid > 0) { int ret, status; ret = rb_waitpid(pid, &status, 0); - if (ret == (rb_pid_t)-1) + if (ret == (rb_pid_t)-1) { + ruby_nocldwait = eargp->nocldwait_prev; + RB_GC_GUARD(execarg_obj); rb_sys_fail("Another thread waited the process started by system()."); + } } #endif - TypedData_Get_Struct(execarg_obj, struct rb_execarg, &exec_arg_data_type, eargp); + ruby_nocldwait = eargp->nocldwait_prev; if (pid < 0) { if (eargp->exception) { int err = errno; Index: test/ruby/test_signal.rb =================================================================== --- test/ruby/test_signal.rb (revision 63878) +++ test/ruby/test_signal.rb (revision 63879) @@ -326,4 +326,47 @@ class TestSignal < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_signal.rb#L326 end end; end + + def test_sigchld_ignore + skip 'no SIGCHLD' unless Signal.list['CHLD'] + old = trap(:CHLD, 'IGNORE') + cmd = [ EnvUtil.rubybin, '--disable=gems', '-e' ] + assert(system(*cmd, 'exit!(0)'), 'no ECHILD') + t0 = Process.clock_gettime(Process::CLOCK_MONOTONIC) + IO.pipe do |r, w| + pid = spawn(*cmd, "STDIN.read", in: r) + nb = Process.wait(pid, Process::WNOHANG) + th = Thread.new(Thread.current) do |parent| + Thread.pass until parent.stop? # wait for parent to Process.wait + w.close + end + assert_raise(Errno::ECHILD) { Process.wait(pid) } + assert_nil nb + end + + IO.pipe do |r, w| + pids = 3.times.map { spawn(*cmd, 'exit!', out: w) } + w.close + zombies = pids.dup + assert_nil r.read(1), 'children dead' + + Timeout.timeout(3) do + zombies.delete_if do |pid| + begin + Process.kill(0, pid) + false + rescue Errno::ESRCH + true + end + end while zombies[0] + end + assert_predicate zombies, :empty?, 'zombies leftover' + + pids.each do |pid| + assert_raise(Errno::ECHILD) { Process.waitpid(pid) } + end + end + ensure + trap(:CHLD, old) + end end -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/