diff --git a/lib/timeout.rb b/lib/timeout.rb index 4281048..9969fa2 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -54,8 +54,6 @@ def self.handle_timeout(message) # :nodoc: private_constant :GET_TIME class State - attr_reader :condvar, :queue, :queue_mutex # shared with Timeout.timeout() - def initialize @condvar = ConditionVariable.new @queue = Queue.new @@ -125,13 +123,20 @@ def ensure_timeout_thread_created # 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 end end end + + def add_request(request) + Sync.synchronize @queue_mutex do + @queue << request + @condvar.signal + end + end end private_constant :State @@ -148,6 +153,7 @@ def initialize(thread, timeout, exception_class, message) @done = false # protected by @mutex end + # Only called by the timeout thread, so does not need Sync.synchronize def done? @mutex.synchronize do @done @@ -158,6 +164,7 @@ def expired?(now) now >= @deadline end + # Only called by the timeout thread, so does not need Sync.synchronize def interrupt @mutex.synchronize do unless @done @@ -168,13 +175,33 @@ def interrupt 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 @@ -263,10 +290,7 @@ def self.timeout(sec, klass = nil, message = nil, &block) #:yield: +sec+ perform = Proc.new do |exc| request = Request.new(Thread.current, sec, exc, message) - state.queue_mutex.synchronize do - state.queue << request - state.condvar.signal - end + state.add_request(request) begin return yield(sec) ensure diff --git a/test/test_timeout.rb b/test/test_timeout.rb index 01beadb..7421b5b 100644 --- a/test/test_timeout.rb +++ b/test/test_timeout.rb @@ -416,4 +416,33 @@ def test_ractor 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