From b49ff7cc700ffdba26fabaaf8167eee189797edf Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 11 Dec 2025 14:16:49 +0100 Subject: [ruby/timeout] Make Timeout.timeout work in a trap handler on CRuby * Fixes https://github.com/ruby/timeout/issues/17 https://github.com/ruby/timeout/commit/1a499a8f96 --- lib/timeout.rb | 28 +++++++++++++++++++++++++--- test/test_timeout.rb | 29 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index 6aa938cdcf..9969fa2e57 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -123,7 +123,7 @@ module Timeout # In that case, just return and let the main thread create the Timeout thread. return if @timeout_thread_mutex.owned? - @timeout_thread_mutex.synchronize do + Sync.synchronize @timeout_thread_mutex do unless @timeout_thread&.alive? @timeout_thread = create_timeout_thread end @@ -132,7 +132,7 @@ module Timeout end def add_request(request) - @queue_mutex.synchronize do + Sync.synchronize @queue_mutex do @queue << request @condvar.signal end @@ -153,6 +153,7 @@ module Timeout @done = false # protected by @mutex end + # Only called by the timeout thread, so does not need Sync.synchronize def done? @mutex.synchronize do @done @@ -163,6 +164,7 @@ module Timeout now >= @deadline end + # Only called by the timeout thread, so does not need Sync.synchronize def interrupt @mutex.synchronize do unless @done @@ -173,13 +175,33 @@ module Timeout end def finished - @mutex.synchronize do + Sync.synchronize @mutex do @done = true end end end private_constant :Request + module Sync + # Calls mutex.synchronize(&block) but if that fails on CRuby due to being in a trap handler, + # run mutex.synchronize(&block) in a separate Thread instead. + def self.synchronize(mutex, &block) + begin + mutex.synchronize(&block) + rescue ThreadError => e + raise e unless e.message == "can't be called from trap context" + # Workaround CRuby issue https://bugs.ruby-lang.org/issues/19473 + # which raises on Mutex#synchronize in trap handler. + # It's expensive to create a Thread just for this, + # but better than failing. + Thread.new { + mutex.synchronize(&block) + }.join + end + end + end + private_constant :Sync + # :startdoc: # Perform an operation in a block, raising an exception if it takes longer than diff --git a/test/test_timeout.rb b/test/test_timeout.rb index 01beadbda6..7421b5ba41 100644 --- a/test/test_timeout.rb +++ b/test/test_timeout.rb @@ -416,4 +416,33 @@ class TestTimeout < Test::Unit::TestCase assert_equal :ok, r end; end if defined?(::Ractor) && RUBY_VERSION >= '4.0' + + def test_timeout_in_trap_handler + # https://github.com/ruby/timeout/issues/17 + + # Test as if this was the first timeout usage + kill_timeout_thread + + rd, wr = IO.pipe + + trap("SIGUSR1") do + begin + Timeout.timeout(0.1) do + sleep 1 + end + rescue Timeout::Error + wr.write "OK" + wr.close + else + wr.write "did not raise" + ensure + wr.close + end + end + + Process.kill :USR1, Process.pid + + assert_equal "OK", rd.read + rd.close + end end -- cgit v1.2.3