From 7e81bf5c0c8f43602e6d901f4253dca2f3d71745 Mon Sep 17 00:00:00 2001 From: Luke Gruber Date: Mon, 5 Jan 2026 12:18:47 -0500 Subject: [PATCH] Fix sleep spurious wakeup from sigchld (#15802) When sleeping with `sleep`, currently the main thread can get woken up from sigchld from any thread (subprocess exited). The timer thread wakes up the main thread when this happens, as it checks for signals. The main thread then executes the ruby sigchld handler if one is registered and is supposed to go back to sleep immediately. This is not ideal but it's the way it's worked for a while. In commit 8d8159e7d8 I added writes to `th->status` before and after `wait_running_turn` in `thread_sched_to_waiting_until_wakeup`, which is called from `sleep`. This is usually the right way to set the thread's status, but `sleep` is an exception because the writes to `th->status` are done in `sleep_forever`. There's a loop that checks `th->status` in `sleep_forever`. When the main thread got woken up from sigchld it saw the changed `th->status` and continued to run the main thread instead of going back to sleep. The following script shows the error. It was returning instead of sleeping forever. ```ruby t = Thread.new do sleep 0.3 `echo hello` # Spawns subprocess puts "Subprocess exited" end puts "Main thread sleeping..." result = sleep # Should block forever puts "sleep returned: #{result.inspect}" ``` Fixes [Bug #21812] --- test/ruby/test_sleep.rb | 18 ++++++++++++++++++ thread_pthread.c | 3 +-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/test/ruby/test_sleep.rb b/test/ruby/test_sleep.rb index 991b73ebd50757..7ef962db4a3b6b 100644 --- a/test/ruby/test_sleep.rb +++ b/test/ruby/test_sleep.rb @@ -1,6 +1,7 @@ # frozen_string_literal: false require 'test/unit' require 'etc' +require 'timeout' class TestSleep < Test::Unit::TestCase def test_sleep_5sec @@ -13,4 +14,21 @@ def test_sleep_5sec assert_operator(slept, :<=, 6.0, "[ruby-core:18015]: longer than expected") end end + + def test_sleep_forever_not_woken_by_sigchld + begin + t = Thread.new do + sleep 0.5 + `echo hello` + end + + assert_raise Timeout::Error do + Timeout.timeout 2 do + sleep # Should block forever + end + end + ensure + t.join + end + end end diff --git a/thread_pthread.c b/thread_pthread.c index a2e42da13e70b1..9c7754067bcdf9 100644 --- a/thread_pthread.c +++ b/thread_pthread.c @@ -1109,10 +1109,9 @@ thread_sched_to_waiting_until_wakeup(struct rb_thread_sched *sched, rb_thread_t { if (!RUBY_VM_INTERRUPTED(th->ec)) { bool can_direct_transfer = !th_has_dedicated_nt(th); - th->status = THREAD_STOPPED_FOREVER; + // NOTE: th->status is set before and after this sleep outside of this function in `sleep_forever` thread_sched_wakeup_next_thread(sched, th, can_direct_transfer); thread_sched_wait_running_turn(sched, th, can_direct_transfer); - th->status = THREAD_RUNNABLE; } else { RUBY_DEBUG_LOG("th:%u interrupted", rb_th_serial(th));