diff --git a/internal.h b/internal.h index 2d71eb178f..45c499d869 100644 --- a/internal.h +++ b/internal.h @@ -1683,6 +1683,7 @@ struct rb_execarg { 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 (0waiting_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 flags) } 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) 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; diff --git a/signal.c b/signal.c index e9f0708510..6393273adf 100644 --- a/signal.c +++ b/signal.c @@ -531,6 +531,7 @@ static struct { 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 handler) #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) 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) case 7: if (memcmp(cptr, "SIG_IGN", 7) == 0) { sig_ign: - if (sig == RUBY_SIGCHLD) { - goto sig_dfl; - } func = SIG_IGN; *cmd = Qtrue; } diff --git a/test/ruby/test_signal.rb b/test/ruby/test_signal.rb index 9003ce1acf..f242a2dc4c 100644 --- a/test/ruby/test_signal.rb +++ b/test/ruby/test_signal.rb @@ -326,4 +326,47 @@ class TestSignal < Test::Unit::TestCase 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