From: merch-redmine@...
Date: 2019-06-11T19:40:51+00:00
Subject: [ruby-core:93050] [Ruby trunk Bug#15645] It is possible to escape `Mutex#synchronize` without releasing the mutex

Issue #15645 has been updated by jeremyevans0 (Jeremy Evans).


lexi.lambda (Alexis King) wrote:
> Therefore, the solution seems simple: **instead of using `rb_mutex_lock` inside of `rb_mutex_synchronize`, use `mutex_lock_uninterruptible`, which sets `interruptible_p` to `FALSE` and avoids the issue entirely.**

With this simple solution:

```diff
--- thread_sync.c
+++ thread_sync.c
@@ -506,7 +506,7 @@ VALUE
 rb_mutex_synchronize(VALUE mutex, VALUE (*func)(VALUE arg), VALUE arg)
 {
     rb_mutex_lock(mutex);
-    return rb_ensure(func, arg, rb_mutex_unlock, mutex);
+    return rb_ensure(func, arg, mutex_lock_uninterruptible, mutex);
 }

 /*

```

There is a deadlock error when building ruby:

```
./miniruby -I./lib -I. -I.ext/common  ./tool/transform_mjit_header.rb "cc  -O0 -g -pipe -fPIC " rb_mjit_header.h .ext/include/x86_64-openbsd6.5/rb_mjit_min_header-2.7.0.h
Traceback (most recent call last):
        7: from ./tool/transform_mjit_header.rb:240:in `<main>'
        6: from ./tool/transform_mjit_header.rb:177:in `conflicting_types?'
        5: from ./tool/transform_mjit_header.rb:188:in `with_code'
        4: from ./lib/tempfile.rb:295:in `open'
        3: from ./tool/transform_mjit_header.rb:189:in `block in with_code'
        2: from ./lib/delegate.rb:349:in `block in delegating_block'
        1: from ./lib/delegate.rb:349:in `puts'
./lib/delegate.rb:349:in `write': deadlock; recursive locking (ThreadError)
*** Error 1 in /. (Makefile:890 '.ext/include/x86_64-openbsd6.5/rb_mjit_min_header-2.7.0.h')
```

I'm not sure how to fix the issue, assuming it is actually possible to fix.  Maybe using the equivalent of `Thread.handle_interrupt(Exception => :never)` around the call to `rb_ensure` and `Thread.handle_interrupt(Exception => :immediate)` around the call to `func` , as implied by the `Thread.handle_interrupt` documentation? I think `Thread#raise` and `Thread#kill` should be avoided if at all possible, and we should probably update the documentation to describe the problems with them and warn against their use.

----------------------------------------
Bug #15645: It is possible to escape `Mutex#synchronize` without releasing the mutex
https://bugs.ruby-lang.org/issues/15645#change-78448

* Author: jneen (Jeanine Adkisson)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.1p33 (2019-01-30 revision 66950) [x86_64-linux]
* Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Hello, I hope this finds you well.

I have a persistent deadlocking issue in a project that relies both on `Mutex#synchronize` and `Thread#raise`, and I believe I have reduced the problem to the following example, in which it is possible to exit a `synchronize` block without releasing the mutex.

``` ruby
mutex = Mutex.new
class E < StandardError; end

t1 = Thread.new do
  10000.times do
    begin
      mutex.synchronize do
        puts 'acquired'
        # sleep 0.01
        raise E if rand < 0.5
        puts 'releasing'
      end
    rescue E
      puts "interrupted"
    end

    puts "UNRELEASED MUTEX" if mutex.owned?
  end
end

t2 = Thread.new do
  1000.times do
    mutex.synchronize { sleep 0.01 }
    sleep 0.01
    t1.raise(E)
  end
end

t3 = Thread.new do
  1000.times do
    mutex.synchronize { sleep 0.01 }
    sleep 0.01
    t1.raise(E)
  end
end

t2.join
t3.join
```

I would expect `mutex.owned?` to always return `false` outside of the `synchronize { ... }` block, but when I run the above script, I see the following output:

```
; ruby tmp/testy.rb
acquired
interrupted
interrupted
UNRELEASED MUTEX
#<Thread:0x00005577aaa07860@tmp/testy.rb:4 run> terminated with exception (report_on_
exception is true):
Traceback (most recent call last):
        3: from tmp/testy.rb:5:in `block in <main>'
        2: from tmp/testy.rb:5:in `times'
        1: from tmp/testy.rb:7:in `block (2 levels) in <main>'
tmp/testy.rb:7:in `synchronize': deadlock; recursive locking (ThreadError)
```

I do not fully understand why this is possible, and it is possible there is a simpler example that would reproduce the issue. But it seems at least that it is necessary for two different threads to be running `Thread#raise` simultaneously.

Occasionally, especially if the timing of the `sleep` calls are tuned, the thread `t1` will display an stack trace for an error `E` - which I believe is the expected behavior in the case that the error is raised during its rescue block.

Thank you for your time!




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