refactor our synchronized delegator to work with MRI/JRuby 1.8 std-lib

this due delegator.rb library doing eval on `initialize` - thus `method_missing` is not actually called for __send__ delegation to the target. we avoid this by unpatching the work done by MRI as well as JRuby in 1.8 mode (which is slightly different since it tries to pre-generate a module that it includes based on target's class do not do that much eval).

changed tests due a bug in delegator.rb that affects 1.8 - we've been expecting a delegated Array#each (with a block) which won't work since the block is not passed to the target ...

Author: kares

lib/thread_safe/synchronized_delegator.rb

@@ -1,35 +1,75 @@
+require 'delegate'
+
 # This class provides a trivial way to synchronize all calls to a given object
-# by wrapping it with a Delegator that performs Mutex#lock/unlock calls around
-# the delegated #send. Example:
+# by wrapping it with a `Delegator` that performs `Mutex#lock/unlock` calls
+# around the delegated `#send`. Example:
 #
 #   array = [] # not thread-safe on many impls
-#   array = MutexedDelegator.new(array) # thread-safe
+#   array = SynchronizedDelegator.new([]) # thread-safe
 #
-# A simple Mutex provides a very coarse-grained way to synchronize a given
+# A simple `Mutex` provides a very coarse-grained way to synchronize a given
 # object, in that it will cause synchronization for methods that have no
 # need for it, but this is a trivial way to get thread-safety where none may
 # exist currently on some implementations.
 #
 # This class is currently being considered for inclusion into stdlib, via
 # https://bugs.ruby-lang.org/issues/8556
+class SynchronizedDelegator < SimpleDelegator
 
-require 'delegate'
+  def initialize(obj)
+    super # __setobj__(obj)
+    @mutex = Mutex.new
+    undef_cached_methods!
+  end
 
-unless defined?(SynchronizedDelegator)
-  class SynchronizedDelegator < SimpleDelegator
-    def initialize(*)
+  def method_missing(method, *args, &block)
+    mutex = @mutex
+    begin
+      mutex.lock
       super
-      @mutex = Mutex.new
+    ensure
+      mutex.unlock
     end
+  end
+
+  private
+
+  if RUBY_VERSION[0, 3] == '1.8'
 
-    def method_missing(m, *args, &block)
-      begin
-        mutex = @mutex
-	mutex.lock
-	super
-      ensure
-	mutex.unlock
+    def singleton_class
+      class << self; self end
+    end unless respond_to?(:singleton_class)
+
+    # The 1.8 delegator library does (instance) "eval" all methods
+    # delegated on {#initialize}.
+    # @see http://rubydoc.info/stdlib/delegate/1.8.7/Delegator
+    # @private
+    def undef_cached_methods!
+      self_class = singleton_class
+      for method in self_class.instance_methods(false)
+        self_class.send :undef_method, method
       end
     end
+
+    # JRuby 1.8 mode stdlib internals - caching generated modules
+    # methods under `Delegator::DelegatorModules` based on class.
+    # @private
+    def undef_cached_methods!
+      gen_mod = DelegatorModules[[__getobj__.class, self.class]]
+      if gen_mod && singleton_class.include?(gen_mod)
+        self_class = singleton_class
+        for method in gen_mod.instance_methods(false)
+          self_class.send :undef_method, method
+        end
+      end
+    end if constants.include?('DelegatorModules')
+
+  else
+
+    # Nothing to do since 1.9 {#method_missing} will get called.
+    # @private
+    def undef_cached_methods!; end
+
   end
-end
+
+end unless defined?(SynchronizedDelegator)

test/test_synchronized_delegator.rb

@@ -2,41 +2,83 @@
 require 'thread_safe/synchronized_delegator.rb'
 
 class TestSynchronizedDelegator < Test::Unit::TestCase
+
   def test_wraps_array
-    ary = []
-    sync_ary = SynchronizedDelegator.new(ary)
+    sync_array = SynchronizedDelegator.new(array = [])
+
+    array << 1
+    assert_equal 1, sync_array[0]
 
-    ary << 1
-    assert_equal 1, sync_ary[0]
+    sync_array << 2
+    assert_equal 2, array[1]
   end
 
   def test_synchronizes_access
-    ary = []
-    sync_ary = SynchronizedDelegator.new(ary)
+    t1_continue, t2_continue = false, false
+
+    hash = Hash.new do |hash, key|
+      t2_continue = true
+      unless hash.find { |e| e[1] == key.to_s } # just to do something
+        hash[key] = key.to_s
+        Thread.pass until t1_continue
+      end
+    end
+    sync_hash = SynchronizedDelegator.new(hash)
+    sync_hash[1] = 'egy'
+
+    t1 = Thread.new do
+      sync_hash[2] = 'dva'
+      sync_hash[3] # triggers t2_continue
+    end
+
+    t2 = Thread.new do
+      Thread.pass until t2_continue
+      sync_hash[4] = '42'
+    end
 
-    t1_continue = false
-    t2_continue = false
+    sleep(0.05) # sleep some to allow threads to boot up
+
+    until t2.status == 'sleep' do
+      Thread.pass
+    end
+
+    assert_equal 3, hash.keys.size
+
+    t1_continue = true
+    t1.join; t2.join
+
+    assert_equal 4, sync_hash.size
+  end
+
+  def test_synchronizes_access_with_block
+    t1_continue, t2_continue = false, false
+
+    sync_array = SynchronizedDelegator.new(array = [])
 
     t1 = Thread.new do
-      sync_ary << 1
-      sync_ary.each do
+      sync_array << 1
+      sync_array.each do
         t2_continue = true
         Thread.pass until t1_continue
       end
     end
 
     t2 = Thread.new do
+      # sleep(0.01)
       Thread.pass until t2_continue
-      sync_ary << 2
+      sync_array << 2
+    end
+
+    until t2.status == 'sleep' || t2.status == false do
+      Thread.pass
     end
 
-    Thread.pass until t2.status == 'sleep'
-    assert_equal 1, ary.size
+    assert_equal 1, array.size
 
     t1_continue = true
-    t1.join
-    t2.join
+    t1.join; t2.join
+
+    assert_equal [1, 2], array
+  end if RUBY_VERSION !~ /1\.8/
 
-    assert_equal 2, sync_ary.size
-  end
 end