From: "byroot (Jean Boussier)" <noreply@...>
Date: 2022-11-21T22:02:59+00:00
Subject: [ruby-core:110846] [Ruby master Feature#19141] Add thread-owned Monitor to protect thread-local resources

Issue #19141 has been updated by byroot (Jean Boussier).


@Eregon how does this relate to `Fiber#storage`?

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

* 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>