Bug #14998: Race conditions in MonitorMixin when interrupted

• Status: Closed
• Assignee: shugo (Shugo Maeda)
• Target version: -
• ruby -v: ruby 2.6.0dev (2018-08-16 trunk 64394) [x86_64-linux]
• Backport: 2.3: UNKNOWN, 2.4: UNKNOWN, 2.5: UNKNOWN

[Eregon (Benoit Daloze)]

From https://bugs.ruby-lang.org/issues/14859#note-9

The code of MonitorMixin#wait is:

    def wait(timeout = nil)
      @monitor.__send__(:mon_check_owner)
      count = @monitor.__send__(:mon_exit_for_cond)
      begin
        @cond.wait(@monitor.instance_variable_get(:@mon_mutex), timeout)
        return true
      ensure
        # What if Thread#raise happens here?
        @monitor.__send__(:mon_enter_for_cond, count)
      end
    end

Probably this code needs to carefully use Thread.handle_interrupt.

[shugo (Shugo Maeda)]

Eregon (Benoit Daloze) wrote:

From https://bugs.ruby-lang.org/issues/14859#note-9

The code of MonitorMixin#wait is:

    def wait(timeout = nil)
      @monitor.__send__(:mon_check_owner)
      count = @monitor.__send__(:mon_exit_for_cond)
      begin
        @cond.wait(@monitor.instance_variable_get(:@mon_mutex), timeout)
        return true
      ensure
        # What if Thread#raise happens here?
        @monitor.__send__(:mon_enter_for_cond, count)
      end
    end

Probably this code needs to carefully use Thread.handle_interrupt.

How about the following code?

    def wait(timeout = nil)
      Thread.handle_interrupt(Exception => :never) do
        @monitor.__send__(:mon_check_owner)
        count = @monitor.__send__(:mon_exit_for_cond)
        begin
          Thread.handle_interrupt(Exception => :immediate) do
            @cond.wait(@monitor.instance_variable_get(:@mon_mutex), timeout)
          end
          return true
        ensure
          @monitor.__send__(:mon_enter_for_cond, count)
        end
      end
    end

[shugo (Shugo Maeda)]

• Status changed from Open to Closed

Applied in changeset trunk|r66061.

---

lib/monitor.rb: avoid race conditions by Thread.handle_interrupt

Suggested by Benoit Daloze. [ruby-core:88502] [Bug #14998]