archive/ruby
1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2025-08-15 13:39:04 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Allow waitpid(-1, Process::WNOHANG) to be woken if a waitpid(pid) is

Browse source 
...pending

If two threads are running, with one calling waitpid(-1, Process::WNOHANG),
and another calling waitpid($some_pid), and then
$some_other_pid exits, we would expect the waitpid(-1,
Process::WNOHANG) call to retrieve that exit status. However, it
cannot actually do so until $some_pid _also_ exits.

This patch fixes the issue by calling do_waitpid unconditionally in
waitpid_wait; this will ensure that a waitpid -1 actually reaps
something (after first checking that no PID-directed call wants the
process).

[Bug #20490]
This commit is contained in:
Stan Hu 2024-05-17 17:05:48 +09:00 • committed by nagachika
parent 50399eebd9
commit 65fed1c3e4
2 changed files with 43 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

24
process.c
View file

@ -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;

30
test/ruby/test_process.rb
View file

@ -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")