move attr_volatile into Concurrent::Synchronization::Volatile module

[colinsurprenant]

add a new Concurrent::Synchronization::Volatile module which can be included into any class to add the attr_volatile class method.

Example:

class Foo
  include Concurrent::Synchronization::Volatile

  attr_volatile :bar

  def initialize
    self.bar = 1
  end
end

foo = Foo.new
foo.bar
=> 1
foo.bar = 2
=> 2

For this the respective {jruby|mri|rbx}_object.rb have been refactored into modules but the original classes were preserved and uses the new modules so not to break backward compatibility.

The Java JRuby synchronization library has been refactored to also add the previous JRubyObject method in the new JRubyAttrVolatile module.

I believe the only risk with the Java synchronization library changes is related to having the volatile threadContext field used as a memory barrier to now be located at the module level and thus will be shared across the including objects using the module. I believe the happens-before / memory barrier is preserved but may be less efficient than having per-object volatile field as memory barrier.

I only tested on JRuby 1.7.22 and all specs are passing.

• add specific specs for the module inclusion
• mri build and run specs
• rbx build and run specs

[pitr-ch]

Thank you for the PR. It was not very well documented, volatile variables should not be ever accessed directly, it bypasses the volatile write/read. I've updated the example. I'll have a closer look tomorrow.

[colinsurprenant]

@pitr-ch yikes, you are totally right, it obviously bypasses the whole volatility system put in place here 😳

[pitr-ch]

Could you use same pattern as Object is using, with the Implementation intermediate.

[colinsurprenant]

not sure exactly what you are suggesting: I see that in Object there is private_constant :ObjectImplementation, but in volatile.rb the Volatile constant must stay public since the it the module you include, which points to the correct module implementation.

or do you mean just adding

    # @!visibility private
    # @!macro internal_implementation_note

to the actual module implementation, like module JRubyAttrVolatile ?

[pitr-ch]

Ah sorry, I did not think it through, it's a module not a class so it is not applicable.

[colinsurprenant]

one thing we could do is to maybe DRY up this switch over the ruby implementations.

[pitr-ch]

Looks good 👍 Please let me know when it's ready, I'll pull it and merge it with my other changes, taking care of conflicts.

[jdantonio]

The tests that failed on the lat Travis build can be ignored with respect to this PR. Those are mine. I've been trying to clean up our tests by removing sleep statements, make them more deterministic, etc. I made changes on master that caused those failures. I believe I have them fixed on the latest master. If they continue to be a problem I'll fix them.

[colinsurprenant]

@jdantonio yeah, thanks, I figured/assumed this much.

I tested on jruby and mri, at the moment I have a hard time building for rbx, but I am comfortable it's ok, since the only real change is on the jruby ext.

@pitr-ch so, I think it's ready, let me know if you'd like me to squash.

[pitr-ch]

@colinsurprenant Thanks! No need for squashing, I'll pull it to my changes and I'll merge it together.

[jdantonio]

@pitr-ch What's the status of this? Any idea when it will be ready to merge?

[pitr-ch]

Sorry, I was busy lately moving. It'll be included in #421, I'll close this PR then.