diff --git a/process.c b/process.c index e8dcb82e50..c317a4cc2c 100644 --- a/process.c +++ b/process.c @@ -1270,6 +1270,7 @@ waitpid_cleanup(VALUE x) return Qfalse; } +#if RUBY_SIGCHLD static void waitpid_wait(struct waitpid_state *w) { @@ -1283,10 +1284,17 @@ waitpid_wait(struct waitpid_state *w) */ rb_native_mutex_lock(&vm->waitpid_lock); - if (w->pid > 0 || ccan_list_empty(&vm->waiting_pids)) { - w->ret = do_waitpid(w->pid, &w->status, w->options | WNOHANG); + if (w->options & WNOHANG && w->pid <= 0) { + /* In the case of WNOHANG wait for a group, make sure there isn't a zombie child + * whose PID we are directly waiting for in another call to waitpid. If there is, + * we will reap it via a call to waitpid($pid) with this call to waitpid_each. */ + waitpid_each(vm, &vm->waiting_pids); + /* _now_ it's safe to call do_waitpid, without risk of stealing the wait from + * another directed call. */ } + w->ret = do_waitpid(w->pid, &w->status, w->options | WNOHANG); + if (w->ret) { if (w->ret == -1) w->errnum = errno; } @@ -1308,6 +1316,7 @@ waitpid_wait(struct waitpid_state *w) rb_ensure(waitpid_sleep, (VALUE)w, waitpid_cleanup, (VALUE)w); } } +#endif static void * waitpid_blocking_no_SIGCHLD(void *x) @@ -1350,12 +1359,11 @@ rb_process_status_wait(rb_pid_t pid, int flags) waitpid_state_init(&waitpid_state, pid, flags); waitpid_state.ec = GET_EC(); - if (WAITPID_USE_SIGCHLD) { - waitpid_wait(&waitpid_state); - } - else { - waitpid_no_SIGCHLD(&waitpid_state); - } +#if WAITPID_USE_SIGCHLD + waitpid_wait(&waitpid_state); +#else + waitpid_no_SIGCHLD(&waitpid_state); +#endif if (waitpid_state.ret == 0) return Qnil; diff --git a/test/ruby/test_process.rb b/test/ruby/test_process.rb index 1228f2c0b1..8e1c3a0d47 100644 --- a/test/ruby/test_process.rb +++ b/test/ruby/test_process.rb @@ -2659,7 +2659,7 @@ EOS end; end if Process.respond_to?(:_fork) - def test_concurrent_group_and_pid_wait + def _test_concurrent_group_and_pid_wait(nohang) # Use a pair of pipes that will make long_pid exit when this test exits, to avoid # leaking temp processes. long_rpipe, long_wpipe = IO.pipe @@ -2681,10 +2681,26 @@ EOS end # Wait for us to be blocking in a call to waitpid2 Thread.pass until t1.stop? + assert_nil Process.waitpid(-1, Process::WNOHANG) short_wpipe.close # Make short_pid exit - # The short pid has exited, so -1 should pick that up. - assert_equal short_pid, Process.waitpid(-1) + # The short pid has exited, so waitpid(-1) should pick that up. + exited_pid = + unless nohang + Process.waitpid(-1) + else + EnvUtil.timeout(5) do + loop do + pid = Process.waitpid(-1, Process::WNOHANG) + + break pid if pid + + sleep 0.1 + end + end + end + + assert_equal short_pid, exited_pid # Terminate t1 for the next phase of the test. t1.kill @@ -2714,6 +2730,14 @@ EOS [long_rpipe, long_wpipe, short_rpipe, short_wpipe].each { _1&.close rescue nil } end if defined?(fork) + def test_concurrent_group_and_pid_wait + _test_concurrent_group_and_pid_wait(false) + end if defined?(fork) + + def test_concurrent_group_and_pid_wait_nohang + _test_concurrent_group_and_pid_wait(true) + end if defined?(fork) + def test_handle_interrupt_with_fork Thread.handle_interrupt(RuntimeError => :never) do Thread.current.raise(RuntimeError, "Queued error")