According to our discussion here https://github.com/rspec/rspec-support/issues/501 it seems like typical implementation of per-thread reentrant mutex is no longer valid and can lead to some deadlock situation.
#!/usr/bin/env ruby
require 'monitor'
def monitor_failure
m = Monitor.new
f1 = Fiber.new do
m.synchronize do
puts "f1 A"
Fiber.yield
puts "f1 B"
end
end
f2 = Fiber.new do
m.synchronize do
puts "f2 A"
# Fiber.yield
f1.resume
puts "f2 B"
end
end
f1.resume
f2.resume
end
monitor_failure
I noticed something else while writing a test for this:
On 2.7:
ruby -e 'm=Mutex.new; m.lock; p 1; Fiber.new { m.lock }.resume'
1
Traceback (most recent call last):
1: from -e:1:in `block in <main>'
-e:1:in `lock': deadlock; recursive locking (ThreadError)
On 3.0:
$ ruby -e 'm=Mutex.new; m.lock; p 1; Fiber.new { m.lock }.resume'
1
-e:1:in `lock': No live threads left. Deadlock? (fatal)
1 threads, 1 sleeps current:0x000000000214f080 main thread:0x000000000214f080
* #<Thread:0x00000000021bb488 sleep_forever>
rb_thread_t:0x000000000214f080 native:0x00007f05edc32b80 int:0
from -e:1:in `block in <main>'
$ ruby -e 'Thread.new {sleep}; sleep 0.1; m=Mutex.new; m.lock; p 1; Fiber.new { m.lock }.resume'
1
=> hangs
Same behavior on master for Monitor.new with my fix (ruby -rmonitor -e 'm=Monitor.new; m.enter; p 1; Fiber.new { p m.enter }.resume' => fatal, or hang if there is another Thread).
It would be nice to raise a ThreadError when detecting in Mutex#lock that the Mutex is owned by the same Thread but a different Fiber, if there is no Fiber scheduler.
That's more of a nice-to-have though.
It would be nice to raise a ThreadError when detecting in Mutex#lock that the Mutex is owned by the same Thread but a different Fiber, if there is no Fiber scheduler.
I just ran into this issue, that would indeed be good to error out rather that deadlock forever.
In other word, if we're trying to enter a monitor currently owned by another fiber belonging to the same thread than us, and there is no fiber scheduler registered.
If so the issue will be to get the thread_id or ptr of a fiber, because monitor is an extension and I don't see anything in the public API that allows to tell that.
One way I guess is I can store rb_thread_current() in the monitor struct, it's a bit redundant but would work I think.