Bug #20490
closedProcess.waitpid2(-1, Process::WNOHANG) misbehaves on Ruby 3.1 & 3.2 with detached process
Description
This is a follow-up issue for a bug that I thought was fixed in https://bugs.ruby-lang.org/issues/19837 and duplicated in https://bugs.ruby-lang.org/issues/20181.
The following script doesn't terminate quickly in Ruby 3.1.5 and 3.2.4, even with the patches to address https://bugs.ruby-lang.org/issues/19837. It works fine in Ruby 3.3. It appears that the Process::WNOHANG
argument passed to Process.wait2
causes this script to spin until the child process stops:
#!/bin/env ruby
Process.spawn({}, "sh -c 'sleep 600'").tap do |pid|
puts "detaching PID #{pid}"
Process.detach(pid)
end
forked_pid = fork do
loop { sleep 1 }
end
child_waiter = Thread.new do
puts "Waiting for child process to die..."
# The spawned process has to exit before this returns in Ruby 3.1 and 3.2
loop do
pid, status = Process.wait2(-1, Process::WNOHANG)
puts "Exited PID: #{pid}, status: #{status}"
break if pid
sleep 1
end
end
process_killer = Thread.new do
puts "Killing #{forked_pid}"
system("kill #{forked_pid}")
end
child_waiter.join
process_killer.join
If I drop the Process::WNOHANG
argument, it works fine.
Updated by stanhu (Stan Hu) 6 months ago ยท Edited
I think this patch in the ruby_3_2
branch fixes the problem:
diff --git a/process.c b/process.c
index 354e16fd26..52d49948a5 100644
--- a/process.c
+++ b/process.c
@@ -1290,7 +1290,7 @@ waitpid_wait(struct waitpid_state *w)
if (w->ret) {
if (w->ret == -1) w->errnum = errno;
}
- else if (w->options & WNOHANG) {
+ else if (w->options & WNOHANG && w->pid > 0) {
}
else {
need_sleep = TRUE;
Updated by stanhu (Stan Hu) 6 months ago
I've opened fixes here:
Updated by nagachika (Tomoyuki Chikanaga) 6 months ago
- Backport changed from 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN to 3.1: WONTFIX, 3.2: REQUIRED, 3.3: DONTNEED
Thank you for your report. I will look the PRs.
I will set the status of this ticket because of this is a backport ticket.
And currently 3.1 branch is under the security maintenance phase, so I fill the Backport field for 3.1 with "WONTFIX". cc: @hsbt (Hiroshi SHIBATA) (as the new 3.1 branch maintainer).
Updated by nagachika (Tomoyuki Chikanaga) 6 months ago
- Status changed from Open to Closed
Updated by nagachika (Tomoyuki Chikanaga) 4 months ago
Currently the backport PR for ruby_3_2 was made by @kjtsanaktsidis (KJ Tsanaktsidis).
https://github.com/ruby/ruby/pull/10787
But in my understanding, it was still in the works.
Updated by kjtsanaktsidis (KJ Tsanaktsidis) 3 months ago
Apologies I missed this - I just rebased the branch, I think it should be OK for a stable backport to 3.2.
Updated by nagachika (Tomoyuki Chikanaga) 3 months ago
- Backport changed from 3.1: WONTFIX, 3.2: REQUIRED, 3.3: DONTNEED to 3.1: WONTFIX, 3.2: DONE, 3.3: DONTNEED
Merged at https://github.com/ruby/ruby/commit/65fed1c3e439bc47bcf6ec884431a86cb9ebd1dc. Thank you!