Atomic get_and_set faulty in C

[pitr-ch]

C version does not compare the read value which allows another update to sneak in.
Compare: https://github.com/ruby-concurrency/concurrent-ruby/blob/master/ext/concurrent/atomic_reference.c#L52-L56 and http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/util/concurrent/atomic/AtomicReference.java#139

[pitr-ch]

Or maybe not, since CRuby is not switching between Threads inside methods implemented in C.

[jdantonio]

@pitr-ch I'll have to research it more. That code was copied from the original gem. My modifications were structural only.

[pitr-ch]

@jdantonio Yeah I know, I'll leave it open for others to comment. The C code is correct if CRuby does not switch threads in C methods (which I think is true). There are 2 actions we could take:

• if I or anybody confirms the switching assumption (which may change in newer version though) we do not need to change anything.
• Do the loop and CAS as in Java to be sure no matter if Ruby does switches in CRuby methods or not.

I would vote for adding the loop and CAS if we do not find any documentation about the assumption.

[jdantonio]

The longer I work on this gem, the more I appreciate languages with written specifications. Even if we can confirm that MRI Ruby does not switch threads in C methods, unless there is a written guarantee somewhere in the Ruby documentation my vote will be to assume the worst and and a loop.

[dsisnero]

Is the c supposed to be used from rubinius. If so, they use real threads

Sent from my iPad

On Jan 27, 2015, at 7:52 AM, jdantonio notifications@github.com wrote:

The longer I work on this gem, the more I appreciate languages with written specifications. Even if we can confirm that MRI Ruby does not switch threads in C methods, unless there is a written guarantee somewhere in the Ruby documentation my vote will be to assume the worst and and a loop.

—
Reply to this email directly or view it on GitHub.

[jdantonio]

@dsisnero The C extensions are not intended to be used under Rubinius. Everything in concurrent-ruby has a pure-Ruby implementation and the C extensions are optional. They will only load under MRI. @brixen has been a great supporter of our work and has indicated that the Rubinius community prefers pure-Ruby implementations.

[brixen]

Rubinius does prefer to expose the necessary facilities to make concurrency models possible in Ruby code. If there is something specific that you need, please let us know. I've been working through this PhD thesis in the interest of improving how Rubinius supports concurrency in Ruby and other languages which target Rubinius.

[pitr-ch]

@brixen thanks. In this case it's fine, we just delegate to Rubinius::AtomicReference :) But it would be useful to have some wiki page dedicated to Rubinius concurrency like JRuby has here. I've found only http://rubini.us/doc/en/systems/concurrency/.

[brixen]

@pitr-ch we will work on documenting everything more thoroughly. Very early sneak preview of upcoming work: https://github.com/rubinius/rubinius-book :)

[pitr-ch]

@brixen Thanks! Stared and watched, looking forward to the concurrency chapter :) The cover is really nice ;)