Make Timeout.timeout work in a trap handler on CRuby

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

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