Isn't MRI 2.0 behaving wrongly here?

output.txt

MRI 2.0.0p353:
$ ruby test.rb 
1
1
102
102

JRuby 1.7.9:
$ ruby test.rb 
19
119
120
120

test.rb

require 'thread'

class A
  attr_reader :a
  @@lock = Mutex.new
  @@incrementer = 0

  def a
    sleep 1
    @a ||= @@lock.synchronize{@@incrementer += 1}
  end

  def incrementer
    @@incrementer
  end
end

a = A.new

100.times.map do
  Thread.start{a.a}
end.each &:join

100.times.map do
  Thread.start{A.new.a}
end.each &:join

p a.a, a.incrementer, A.new.a, a.incrementer

It's hard to show contention for ||= because generally very few threads will have the opportunity to do it wrong.

The first case, a.a, might only be contended for the first couple threads. However, since MRI usually yields to a newly-started thread immediately, it's unlikely any other threads will hit the code and make it non-atomic.

The second case uses a new object for each thread, so there's no contention by default.

It does, however, show inconsistency on my system.

system ~/projects/jruby $ ruby2.1 blah.rb
1
1
102
102

This is correct output if ||= were actually atomic. One increment for the first successful a.a, 100 for the A.new.a loop, and another one for the last A.new.a call.

I usually see this, but not always...

system ~/projects/jruby $ ruby2.1 blah.rb
5
5
106
106

This result shows that you can't guarantee the first thread will be the one to assign a.@A and also shows that the LHS is getting run by multiple threads, since the end total is higher than it should be. In fact, it adds up perfectly; the a.a value is 4 higher than it should be and the final incrementer value is also 4 higher than it should be. The incrementer was run 4 times more than it should have been had this been an atomic operation. All four of those ended up assigning @A.

system ~/projects/jruby $ ruby2.1 blah.rb
3
3
104
104

A similar case.

Interesting. I should try running it more times. But shouldn't the results be more like 1, 101, 102, 102? Isn't this a bug in MRI?

Yeah that's really confusing. In the same line, a.incrementer produces wildly different values.

Exactly :-)

I suspect you're seeing a side effect of the way that ||=, specifically, is expanded by Ruby. That is, a ||= 42 is equivalent to a || a = 42 and not a = a || 42. So, in this case, the critical line is expanded to:

@a || @a = @@lock.synchronize{@@incrementer += 1}

...and, IIRC, since the "or" and the assignment are individual expressions, the GVL will lock around them independently. So, starting multiple threads simultaneously, it's possible that more than one will get through @a || before the first one gets to @a = ..., so multiple threads will execute the rhs of the "or".

@jballanc that part I understand although I suspected MRI threads wouldn't be interrupted in that scenario. I'm mostly confused by the output in MRI as I'd expect it to be similar to JRuby 's.

Array#join ignores the given block.
Your code doesn't wait any threads.

Hi @nobu, sorry I don't understand why you think so. Would you mind to further explain why you think the code won't wait for all threads to join?

Because 100.times.map do end returns an Array.

And Array#join ignores the block. I really have to fix that...

Replace join &:join by each &:join and you get 1, 101, 102, 102

Hi @nobu, @marcandre. Yes, sorry, I intended to write "each &:join", not "join &:join". You're right!

@headius, I always see the same result with the fixed code in MRI (1, 101, 102, 102). Could you please try it fixed and see if you still get those other results? I still believe @A ||= xxx will be mostly atomic in MRI. At least 99.999% of the times... That's why so many bugs only seem to manifest in JRuby and that's why I proposed JRuby to perform the assign atomically automatically.

Anyone replying to me could please add "@rosenfeld" in the comment so that I get notified?

@rosenfeld Now the problem is that the RHS of the ||= is not substantial enough for threads to context switch.

MRI does not run threads concurrently. Each thread gets a slice of time to execute before it gets put back into the queue and another thread gets a chance to run. In this case, the #a method's logic runs easily within that timeslice.

If we replace the #a method with one that forces a thread context switch, MRI fails to do the assigns atomically just like JRuby:

  def a
    sleep 1
    @a ||= begin; Thread.pass; new_a = @@lock.synchronize{@@incrementer += 1}; Thread.pass; new_a; end
  end

This gives me a "bad" result immediately:

$ rvm ruby-2.1 do ruby blah.rb
2
102
103
103

This may seem artificial, but if the RHS had a bit more work to do, like calculating an expensive value or making a database connection, it would be very easy for MRI to schedule another thread while that code is still running.

Trust me...there's absolutely nothing guaranteeing that ||= runs atomically on MRI.