Skip to content

Instantly share code, notes, and snippets.

@rosenfeld
Last active December 31, 2015 22:59
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 1 You must be signed in to fork a gist
  • Save rosenfeld/8056742 to your computer and use it in GitHub Desktop.
Save rosenfeld/8056742 to your computer and use it in GitHub Desktop.
Isn't MRI 2.0 behaving wrongly here?
MRI 2.0.0p353:
$ ruby test.rb
1
1
102
102
JRuby 1.7.9:
$ ruby test.rb
19
119
120
120
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
@rosenfeld
Copy link
Author

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

@nobu
Copy link

nobu commented Dec 23, 2013

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

@rosenfeld
Copy link
Author

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?

@nobu
Copy link

nobu commented Jan 2, 2014

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

@marcandre
Copy link

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

@rosenfeld
Copy link
Author

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?

@headius
Copy link

headius commented Jan 6, 2014

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment