Merge remote-tracking branch 'upstream/pr/424' into synchronization

* upstream/pr/424: (5 commits)
  correct example to use volatile method
  ...

Author: pitr-ch

ext/com/concurrent_ruby/ext/SynchronizationLibrary.java

@@ -6,6 +6,7 @@
 import org.jruby.RubyClass;
 import org.jruby.RubyModule;
 import org.jruby.RubyObject;
+import org.jruby.RubyBasicObject;
 import org.jruby.anno.JRubyClass;
 import org.jruby.anno.JRubyMethod;
 import org.jruby.runtime.ObjectAllocator;
@@ -47,6 +48,9 @@ public void load(Ruby runtime, boolean wrap) throws IOException {
                 defineModule("Concurrent").
                 defineModuleUnder("Synchronization");
 
+        RubyModule jrubyAttrVolatileModule = synchronizationModule.defineModuleUnder("JRubyAttrVolatile");
+        jrubyAttrVolatileModule.defineAnnotatedMethods(JRubyAttrVolatile.class);
+
         defineClass(runtime, synchronizationModule, "AbstractObject", "JRubyObject",
                 JRubyObject.class, JRUBY_OBJECT_ALLOCATOR);
 
@@ -81,21 +85,16 @@ private RubyClass defineClass(Ruby runtime, RubyModule namespace, String parentN
     //   SynchronizedVariableAccessor wraps with synchronized block, StampedVariableAccessor uses fullFence or
     //   volatilePut
 
-    @JRubyClass(name = "JRubyObject", parent = "AbstractObject")
-    public static class JRubyObject extends RubyObject {
-        private static volatile ThreadContext threadContext = null;
-
-        public JRubyObject(Ruby runtime, RubyClass metaClass) {
-            super(runtime, metaClass);
-        }
+    // module JRubyAttrVolatile
+    public static class JRubyAttrVolatile {
 
-        @JRubyMethod
-        public IRubyObject initialize(ThreadContext context) {
-            return this;
-        }
+        // volatile threadContext is used as a memory barrier per the JVM memory model happens-before semantic
+        // on volatile fields. any volatile field could have been used but using the thread context is an
+        // attempt to avoid code elimination.
+        private static volatile ThreadContext threadContext = null;
 
         @JRubyMethod(name = "full_memory_barrier", visibility = Visibility.PRIVATE)
-        public IRubyObject fullMemoryBarrier(ThreadContext context) {
+        public static IRubyObject fullMemoryBarrier(ThreadContext context, IRubyObject self) {
             // Prevent reordering of ivar writes with publication of this instance
             if (UnsafeHolder.U == null || !UnsafeHolder.SUPPORTS_FENCES) {
                 // Assuming that following volatile read and write is not eliminated it simulates fullFence.
@@ -109,35 +108,43 @@ public IRubyObject fullMemoryBarrier(ThreadContext context) {
         }
 
         @JRubyMethod(name = "instance_variable_get_volatile", visibility = Visibility.PROTECTED)
-        public IRubyObject instanceVariableGetVolatile(ThreadContext context, IRubyObject name) {
+        public static IRubyObject instanceVariableGetVolatile(ThreadContext context, IRubyObject self, IRubyObject name) {
             // Ensure we ses latest value with loadFence
             if (UnsafeHolder.U == null || !UnsafeHolder.SUPPORTS_FENCES) {
                 // piggybacking on volatile read, simulating loadFence
                 final ThreadContext oldContext = threadContext;
-                return instance_variable_get(context, name);
+                return ((RubyBasicObject)self).instance_variable_get(context, name);
             } else {
                 UnsafeHolder.loadFence();
-                return instance_variable_get(context, name);
+                return ((RubyBasicObject)self).instance_variable_get(context, name);
             }
         }
 
         @JRubyMethod(name = "instance_variable_set_volatile", visibility = Visibility.PROTECTED)
-        public IRubyObject InstanceVariableSetVolatile(ThreadContext context, IRubyObject name, IRubyObject value) {
+        public static IRubyObject InstanceVariableSetVolatile(ThreadContext context, IRubyObject self, IRubyObject name, IRubyObject value) {
             // Ensure we make last update visible
             if (UnsafeHolder.U == null || !UnsafeHolder.SUPPORTS_FENCES) {
                 // piggybacking on volatile write, simulating storeFence
-                final IRubyObject result = instance_variable_set(name, value);
+                final IRubyObject result = ((RubyBasicObject)self).instance_variable_set(name, value);
                 threadContext = context;
                 return result;
             } else {
                 // JRuby uses StampedVariableAccessor which calls fullFence
                 // so no additional steps needed.
                 // See https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/runtime/ivars/StampedVariableAccessor.java#L151-L159
-                return instance_variable_set(name, value);
+                return ((RubyBasicObject)self).instance_variable_set(name, value);
             }
         }
     }
 
+    @JRubyClass(name = "JRubyObject", parent = "AbstractObject")
+    public static class JRubyObject extends RubyObject {
+
+        public JRubyObject(Ruby runtime, RubyClass metaClass) {
+            super(runtime, metaClass);
+        }
+    }
+
     @JRubyClass(name = "Object", parent = "JRubyObject")
     public static class Object extends JRubyObject {
 

lib/concurrent/synchronization.rb

@@ -7,6 +7,7 @@
 require 'concurrent/synchronization/jruby_object'
 require 'concurrent/synchronization/rbx_object'
 require 'concurrent/synchronization/object'
+require 'concurrent/synchronization/volatile'
 
 require 'concurrent/synchronization/abstract_lockable_object'
 require 'concurrent/synchronization/mri_lockable_object'

lib/concurrent/synchronization/jruby_object.rb

@@ -3,29 +3,41 @@ module Synchronization
 
     if Concurrent.on_jruby?
 
-      # @!visibility private
-      # @!macro internal_implementation_note
-      class JRubyObject < AbstractObject
+      module JRubyAttrVolatile
+        def self.included(base)
+          base.extend(ClassMethods)
+        end
 
-        def self.attr_volatile(*names)
-          names.each do |name|
+        module ClassMethods
+          def attr_volatile(*names)
+            names.each do |name|
 
-            ivar = :"@volatile_#{name}"
+              ivar = :"@volatile_#{name}"
 
-            class_eval <<-RUBY, __FILE__, __LINE__ + 1
-              def #{name}
-                instance_variable_get_volatile(:#{ivar})
-              end
+              class_eval <<-RUBY, __FILE__, __LINE__ + 1
+                def #{name}
+                  instance_variable_get_volatile(:#{ivar})
+                end
 
-              def #{name}=(value)
-                instance_variable_set_volatile(:#{ivar}, value)
-              end
-            RUBY
+                def #{name}=(value)
+                  instance_variable_set_volatile(:#{ivar}, value)
+                end
+              RUBY
 
+            end
+            names.map { |n| [n, :"#{n}="] }.flatten
           end
-          names.map { |n| [n, :"#{n}="] }.flatten
         end
+      end
 
+      # @!visibility private
+      # @!macro internal_implementation_note
+      class JRubyObject < AbstractObject
+        include JRubyAttrVolatile
+
+        def initialize
+          # nothing to do
+        end
       end
     end
   end

lib/concurrent/synchronization/mri_object.rb

@@ -1,35 +1,43 @@
 module Concurrent
   module Synchronization
 
-    # @!visibility private
-    # @!macro internal_implementation_note
-    class MriObject < AbstractObject
+    module MriAttrVolatile
+      def self.included(base)
+        base.extend(ClassMethods)
+      end
 
-      def initialize
-        # nothing to do
+      module ClassMethods
+        def attr_volatile(*names)
+          names.each do |name|
+            ivar = :"@volatile_#{name}"
+            class_eval <<-RUBY, __FILE__, __LINE__ + 1
+              def #{name}
+                #{ivar}
+              end
+
+              def #{name}=(value)
+                #{ivar} = value
+              end
+            RUBY
+          end
+          names.map { |n| [n, :"#{n}="] }.flatten
+        end
       end
 
       def full_memory_barrier
         # relying on undocumented behavior of CRuby, GVL acquire has lock which ensures visibility of ivars
         # https://github.com/ruby/ruby/blob/ruby_2_2/thread_pthread.c#L204-L211
       end
+    end
 
-      def self.attr_volatile(*names)
-        names.each do |name|
-          ivar = :"@volatile_#{name}"
-          class_eval <<-RUBY, __FILE__, __LINE__ + 1
-            def #{name}
-          #{ivar}
-            end
+    # @!visibility private
+    # @!macro internal_implementation_note
+    class MriObject < AbstractObject
+      include MriAttrVolatile
 
-            def #{name}=(value)
-              #{ivar} = value
-            end
-          RUBY
-        end
-        names.map { |n| [n, :"#{n}="] }.flatten
+      def initialize
+        # nothing to do
       end
     end
-
   end
 end

lib/concurrent/synchronization/rbx_object.rb

@@ -1,36 +1,45 @@
 module Concurrent
   module Synchronization
 
-    # @!visibility private
-    # @!macro internal_implementation_note
-    class RbxObject < AbstractObject
-      def initialize
-        # nothing to do
+    module RbxAttrVolatile
+      def self.included(base)
+        base.extend(ClassMethods)
+      end
+
+      module ClassMethods
+        def attr_volatile(*names)
+          names.each do |name|
+            ivar = :"@volatile_#{name}"
+            class_eval <<-RUBY, __FILE__, __LINE__ + 1
+              def #{name}
+                Rubinius.memory_barrier
+                #{ivar}
+              end
+
+              def #{name}=(value)
+                #{ivar} = value
+                Rubinius.memory_barrier
+              end
+            RUBY
+          end
+          names.map { |n| [n, :"#{n}="] }.flatten
+        end
       end
 
       def full_memory_barrier
         # Rubinius instance variables are not volatile so we need to insert barrier
         Rubinius.memory_barrier
       end
+    end
 
-      def self.attr_volatile *names
-        names.each do |name|
-          ivar = :"@volatile_#{name}"
-          class_eval <<-RUBY, __FILE__, __LINE__ + 1
-            def #{name}
-              Rubinius.memory_barrier
-              #{ivar}
-            end
+    # @!visibility private
+    # @!macro internal_implementation_note
+    class RbxObject < AbstractObject
+      include RbxAttrVolatile
 
-            def #{name}=(value)
-              #{ivar} = value
-              Rubinius.memory_barrier
-            end
-          RUBY
-        end
-        names.map { |n| [n, :"#{n}="] }.flatten
+      def initialize
+        # nothing to do
       end
     end
-
   end
 end

lib/concurrent/synchronization/volatile.rb

@@ -0,0 +1,35 @@
+module Concurrent
+  module Synchronization
+
+    # Volatile adds the attr_volatile class method when included.
+    #
+    # @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
+
+    Volatile = case
+    when Concurrent.on_cruby?
+      MriAttrVolatile
+    when Concurrent.on_jruby?
+      JRubyAttrVolatile
+    when Concurrent.on_rbx?
+      RbxAttrVolatile
+    else
+      warn 'Possibly unsupported Ruby implementation'
+      MriAttrVolatile
+    end
+  end
+end