From: "headius (Charles Nutter)" <headius@...>
Date: 2013-10-23T04:07:43+09:00
Subject: [ruby-core:57977] [CommonRuby - Feature #8556] MutexedDelegator as	a trivial way to make an object thread-safe


Issue #8556 has been updated by headius (Charles Nutter).


=begin
ko1 (Koichi Sasada) wrote:
> (1) Try gem first

We could certainly put this into thread_safe gem, which is now a dependency of Rails and pretty widely deployed as a result. I am not opposed to testing this more in the wild before incorporation into stdlib.

So the rest of this may be moot, but I'll proceed anyway.

> (2) Misleading design
> 
> I'm afraid that this library introduce bugs under misunderstanding.
> 
> For example, people consider that this object is without worry about
> synchronization, people may write the following program.

Someone else raised a concern about += and friends, but there's *no* way in a library to ever make those operations thread safe (actually, atomic). That's what the "atomic" gem provides.

The only way to ever make +=, ||=, and others be atomic in Ruby proper would be to change the way they're parsed and potentially add a method that could be called. But this is even unpredictable because for variables, there's still no way to do it atomically.

FWIW, Java's ++ and -- and += and friends are *also* not atomic.

I don't believe these features being non-atomic is a good enough justification to prevent the addition of a synchronized delegator. The sync delegator explicitly just makes individual method calls synchronized; and += and friends require multiple method calls.

> At first I see this result, I can't understand why.
> Of course, this program is completely bad program.
> It is completely my mistake.
> 
> But I think this design will lead such misunderstanding and bugs easily.

But this is not possible to fix in current Ruby and all other languages I know don't guarantee any atomicity here either.

> To avoid a such bug, I define the inc() and sub() method in Array.

This is an appropriate way to do it, indeed. However, anyone else still doing += mess up the results. If you want atomic mutation of individual elements, we need an AtomicArray or similar.

> This works completely.
> 
> But a person who assumes sdel_ary is free from consideration about locking,
> can write the following program:

This is perhaps a valid concern. SynchronizedDelegate could use a method_added hook to wrap new methods, however. Is it warranted?

  class << SynchronizedDelegator
    def method_added(name)
      unsync_name = :"__unsynchronized_#{name}"
      alias_method unsync_name, name
      define_method name do |*args, &block|+  def method_missing(m, *args, &block)
        begin
          monitor = @monitor
          monitor.mon_enter
          __send__ unsync_name, args, block
        ensure
          monitor.mon_exit
        end
      end
    end
  end

Or something like that.

> Maybe professional about concurrency program can avoid such silly bugs.
> But if we introduce it as standard library, I'm afraid they are not.

I don't claim this solution solves all problems, obviously. But it solves many of them. It is an incremental tool to help improve concurrency capabilities of Ruby.

> (3) Lock based thraed programming
> 
> This is my opinion. So it is weak objection for this proposal.
> 
> I believe lock based thread programming introduced many bugs.
> (Synchronized) Queue or more high-level structures should be used.
> 
> Or use Mutex (or monitor) explicitly for fing-grain locking.
> It bothers programmers, so programmers use other approachs such as Queue (I hope).

Getting explicitly concurrency-friendly collections into stdlib would be great. But this was intended as a small step given that 2.1 is close to finished.

Another data point: Java for years has had java.util.Collections.synchronized{List,Map,Set} for doing a quick and easy wrapper around those collection types. Sometimes it's the best simple solution for making a collection thread-safe.
=end
----------------------------------------
Feature #8556: MutexedDelegator as a trivial way to make an object thread-safe
https://bugs.ruby-lang.org/issues/8556#change-42552

Author: headius (Charles Nutter)
Status: Assigned
Priority: Normal
Assignee: ko1 (Koichi Sasada)
Category: 
Target version: Ruby 2.1.0


=begin
I propose adding (({MutexedDelegator})) as a simple way to wrap any object with a thread-safe wrapper, via existing delegation logic in ((%delegate.rb%)).

(({Delegator})) provides a way to pass method calls through to a wrapped object. (({SimpleDelegator})) is a trivial implementation that just holds the object in an instance variable. (({MutexedDelegator})) would extend (({SimpleDelegator})) and only override initialize and (({method_missing})) as follows:

  class MutexedDelegator < SimpleDelegator
    def initialize(*)
      super
      @mutex = Mutex.new
    end
    
    def method_missing(m, *args, &block)
      target, mutex = self.__getobj__, @mutex
      begin
        mutex.lock
        target.__send__(m, *args, &block)
      ensure
        mutex.unlock
      end
    end
  end

The only changes here are:

* (({Mutex#lock})) and (({unlock})) logic wrapping the send
* No (({respond_to?})) check; I'm not sure why it's there to begin with, since if we're in (({method_missing})) the (({super()})) call will fail just like a normal (({method_missing})) failure anyway
* No backtrace manipulation. This does not work on JRuby and Rubinius anyway, and in this case I feel that the delegator should not hide itself, since there's real behavior change happening.

This is a trivial addition to stdlib that would make it simple to synchronize all calls to a given object in the same way as the JDK's (({Collections.synchronizedSet}))/(({Map}))/(({List})) calls.
=end



-- 
http://bugs.ruby-lang.org/