TruffleRuby-specific modifications of the MonitorMixin cause unexpected behavior

[meineerde]

Consider the following example class:

require 'monitor'

class Example
  include MonitorMixin

  def initialize(*array)
    mon_initialize
    @array = array
  end

  def modify
    synchronize do
      @array.map! { |e| e + 1 }
    end
  end

  def to_a
    synchronize { @array.dup }
  end

  private

  def initialize_copy(other)
    # Initialize a new mutex in the copy
    mon_initialize

    # Add copied state while ensuring it can be safely added here in any case
    synchronize do
      @array = other.to_a
    end
  end
end

When initializing an object and subsequently dup'ing it, TriffleRuby unexpectedly raises a deadlock:

example = Example.new(1, 2, 3)
copy = example.dup
# ThreadError: deadlock; recursive locking
# 	from /Users/User/.rubies/truffleruby-1.0.0-rc6/lib/mri/monitor.rb:187:in `lock'
# 	from /Users/User/.rubies/truffleruby-1.0.0-rc6/lib/mri/monitor.rb:187:in `mon_enter'
# 	from /Users/User/.rubies/truffleruby-1.0.0-rc6/lib/mri/monitor.rb:212:in `mon_synchronize'
# 	from (irb):18:in `to_a'
# 	from (irb):29:in `block in initialize_copy'
# 	from /Users/User/.rubies/truffleruby-1.0.0-rc6/lib/mri/monitor.rb:214:in `mon_synchronize'
# 	from (irb):28:in `initialize_copy'
# 	from (irb):34:in `initialize_dup'
# 	from (irb):34:in `dup'
# 	from (irb):34
# 	from /Users/User/.rubies/truffleruby-1.0.0-rc6/bin/irb:29:in `<main>'

In TruffleRuby, the mon_initialize call in initialize_copy is effectively a no-op following d675e82#diff-58dfe1911468f7ccd5bf9bed448b7b48. When copying a monitor, I find it rather surprising that the copy silently uses the original mutex instead of actually initializing a new one.

The example might be a bit contrived, but the issue / bug would still manifest itself later given that the two objects still share the same mutex and can thus not operate concurrently.

Thus, I request that this TruffleRuby specific modification of the MonitorMixin is dropped from TruffleRuby to restore MRI-compatible behavior.

[meineerde]

The original change was apparently introduced in #136, specifically in 464a137:

Inherently unsafe

[eregon]

Thank you for the report.
FWIW, my original commit tried to prevent it more directly: 1adf384, but that made Rails tests fail :(

Anyway, we did not expect mon_initialize to be called from initialize_copy, so we will fix this case.
Obviously, this bug is not intended, no need to "request" changes 😉

[meineerde]

There are probably a lot of other cases too where mon_initialize might be called again with the expectation that it always sets a new mutex, e.g. when using "creative" ways to build objects.

Isn't it possible to completely retain MRI's behavior and always initialize the monitor (again) when mon_initialize is called? I think it's the caller's responsibility to only do this when necessary and intended.

[meineerde]

Thank you both for the extremely quick turnaround! <3