From: "ioquatix (Samuel Williams)" <noreply@...>
Date: 2022-11-22T05:10:07+00:00
Subject: [ruby-core:110849] [Ruby master Feature#19141] Add thread-owned Monitor to protect thread-local resources

Issue #19141 has been updated by ioquatix (Samuel Williams).


Why do you need a recursive mutex for this code? Why not just lock inside the enumerator around whatever shared resource you are protecting?

http://www.zaval.org/resources/library/butenhof1.html

> 1) The biggest of all the big problems with recursive mutexes is that 
they encourage you to completely lose track of your locking scheme and 
scope. This is deadly. Evil. It's the "thread eater". You hold locks for 
the absolutely shortest possible time. Period. Always. If you're calling 
something with a lock held simply because you don't know it's held, or 
because you don't know whether the callee needs the mutex, then you're 
holding it too long. You're aiming a shotgun at your application and 
pulling the trigger. You presumably started using threads to get 
concurrency; but you've just PREVENTED concurrency. 

----------------------------------------
Feature #19141: Add thread-owned Monitor to protect thread-local resources
https://bugs.ruby-lang.org/issues/19141#change-100207

* Author: wildmaples (Maple Ong)
* Status: Open
* Priority: Normal
----------------------------------------
### Background 

In Ruby v3.0.2, Monitor was modified to be owned by fibers instead of threads [for reasons as described in this issue](https://bugs.ruby-lang.org/issues/17827) and so it is also consistent with Mutex. Before the change to Monitor, Mutex was modified to per-fiber instead of thread ([issue](https://bugs.ruby-lang.org/issues/16792), [PR](https://github.com/ruby/ruby/commit/178c1b0922dc727897d81d7cfe9c97d5ffa97fd9)) which caused some problems (See: [comment](https://bugs.ruby-lang.org/issues/17827#note-1)).  

### Problem

We are now encountering a problem where using Enumerator (implemented transparently using Fiber, so the user is not aware) within a Fiber-owned process, which causes a deadlock. That means any framework using Monitor is incompatible to be used with Enumerator. 

In general, there are many types of thread-local resources (connections for example), so it would make sense to have a thread-owned monitor to protect them. I think few resources are really fiber-owned.

#### Specifics 
* Concurrent Ruby is still designed with per-thread locking, which causes similar incompatibilities. (Read: [issue](https://github.com/ruby-concurrency/concurrent-ruby/issues/962))
* Systems test in Rails implements locking using Monitor, resulting in deadlock in these known cases:
  * when cache clearing (Read: [issue](https://github.com/rails/rails/issues/45994))
  * database transactions when used with Enumerator (Read: [comment](https://github.com/rails/rails/issues/45994#issuecomment-1304306575)) 

### Demo

```ruby
# ruby 2.7.6p219 (2022-04-12 revision c9c2245c0a) [arm64-darwin21]
# Thread #<Thread:0x000000014a8eb228 demo.rb:8 run>, fiber #<Fiber:0x000000014a8eaf80 (resumed)>, locked true, owned true
# Thread #<Thread:0x000000014a8eb228 demo.rb:8 run>, fiber #<Fiber:0x000000014a8eacb0 demo.rb:13 (resumed)>, locked true, owned true

# ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21]
# Thread #<Thread:0x0000000102329a08 demo.rb:8 run>, fiber #<Fiber:0x0000000102329828 (resumed)>, locked true, owned true
# Thread #<Thread:0x0000000102329a08 demo.rb:8 run>, fiber #<Fiber:0x00000001023294e0 demo.rb:13 (resumed)>, locked true, owned false

require 'fiber'
require 'monitor'

puts RUBY_DESCRIPTION

# We have a single monitor - we're pretending it protects some thread-local resources
m = Monitor.new

# We'll create an explicit thread
t = Thread.new do
  # Lock to protect our thread-local resource
  m.enter

  puts "Thread #{Thread.current}, fiber #{Fiber.current}, locked #{m.mon_locked?}, owned #{m.mon_owned?}"

  # The Enumerator here creates a fiber, which runs on the same thread, so would want to use the same thread-local resource
  e = Enumerator.new do |y|
    # In 2.7 this is fine, in 3.0 it's not, as the fiber thinks it doesn't have the lock
    puts "Thread #{Thread.current}, fiber #{Fiber.current}, locked #{m.mon_locked?}, owned #{m.mon_owned?}"
    
    # This would deadlock
    # m.enter

    y.yield 1
  end
  e.next
end

t.join
```

### Possible Solutions

* Allow `Monitor` to be per thread or fiber through a flag
* Having `Thread::Monitor` and `Fiber::Monitor` as two separate classes. Leave `Monitor` as it is right now. However, this may not be possible due to the `Thread::Mutex` alias 

These options would give us more flexibility in which type of Monitor to use. 



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>