[GR-18163] Fix usage of Thread.handle_interrupt in MonitorMixin#mon_synchronize

PullRequest: truffleruby/2468

Author: eregon

CHANGELOG.md

@@ -17,6 +17,7 @@ Bug fixes:
 * Fix handling of signals with `--single-threaded` (#2265).
 * Fix `Enumerator::Lazy#{chunk_while, slice_before, slice_after, slice_when}` to return instances of `Enumerator::Lazy` (#2273).
 * Fix `Truffle::Interop.source_location` to return unavailable source sections for modules instead of null (#2257).
+* Fix usage of `Thread.handle_interrupt` in `MonitorMixin#mon_synchronize`.
 
 Compatibility:
 

lib/mri/monitor.rb

@@ -230,11 +230,15 @@ def mon_owned?
   def mon_synchronize
     # Prevent interrupt on handling interrupts; for example timeout errors
     # it may break locking state.
-    Thread.handle_interrupt(EXCEPTION_NEVER){ mon_enter }
-    begin
-      yield
-    ensure
-      Thread.handle_interrupt(EXCEPTION_NEVER){ mon_exit }
+    Thread.handle_interrupt(EXCEPTION_NEVER) do
+      mon_enter
+      begin
+        Thread.handle_interrupt(EXCEPTION_IMMEDIATE) do
+          yield
+        end
+      ensure
+        mon_exit
+      end
     end
   end
   alias synchronize mon_synchronize

spec/ruby/library/monitor/synchronize_spec.rb

@@ -0,0 +1,30 @@
+require_relative '../../spec_helper'
+require 'monitor'
+
+describe "Monitor#synchronize" do
+  it "unlocks after return, even if it was interrupted by Thread#raise" do
+    exc_class = Class.new(RuntimeError)
+
+    monitor = Monitor.new
+    10.times do
+      locked = false
+
+      thread = Thread.new do
+        begin
+          monitor.synchronize do
+            locked = true
+            # Do not wait here, we are trying to interrupt the ensure part of #synchronize
+          end
+          sleep # wait for exception if it did not happen yet
+        rescue exc_class
+          monitor.should_not.mon_locked?
+          :ok
+        end
+      end
+
+      Thread.pass until locked
+      thread.raise exc_class, "interrupt"
+      thread.value.should == :ok
+    end
+  end
+end