https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112013-06-22T01:37:44ZRuby Issue Tracking SystemRuby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=400752013-06-22T01:37:44Zheadius (Charles Nutter)headius@headius.com
<ul></ul><p>A few updates...</p>
<ul>
<li>After experimenting, I realize that the respond_to? check is to trigger "private" error for private methods. It may also have something to do with respond_to_missing? methods.</li>
<li>It is probably better to delegate the dispatching logic to the super impl (in Delegator).</li>
<li>The name "SynchronizedDelegator" was suggested to me as being more indicative of the intent of this logic.</li>
</ul>
<p>So I have a revised implementation:</p>
<pre><code>class SynchronizedDelegator < SimpleDelegator
def initialize(*)
super
@mutex = Mutex.new
end
def method_missing(m, *args, &block)
begin
mutex = @mutex
mutex.lock
super
ensure
mutex.unlock
end
end
end
</code></pre> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=420342013-09-27T19:59:45Zheadius (Charles Nutter)headius@headius.com
<ul></ul><p>Any comments here? This would be pretty easy to add to delegate.rb for 2.1.</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=420382013-09-27T20:13:40Zsimonx1 (Szymon Kurcab)szymon.kurcab@gmail.com
<ul></ul><p>That would be a useful feature.<br>
+1</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=420462013-09-27T20:35:30Zheadius (Charles Nutter)headius@headius.com
<ul></ul><p>Similar in nature to the "synchronized" module method proposed in <a href="https://bugs.ruby-lang.org/issues/8961" class="external">https://bugs.ruby-lang.org/issues/8961</a>. I like that proposal as well, but it does not help the case where you have a concurrency-unsafe object in hand that you would like to make concurrency-safe.</p>
<p>Another commenter on Twitter suggested that this is not the best way to go about making an object thread-safe, and he's right. It would be better to use immutable collections or explicitly concurrency-friendly collections. However, this is a simple pattern that works for all types of objects and makes it possible to start writing better threaded Ruby code today.</p>
<p>I would also like to note that this is helpful in MRI too, since the bodies of Ruby methods can context switch at any time. MRI also needs a better mechanism for saying "this class's methods should only be executed by one thread at a time".</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=420562013-09-28T03:42:27Zheadius (Charles Nutter)headius@headius.com
<ul></ul><p>Formatting issue... the "synchronized" proposal is in <a href="https://bugs.ruby-lang.org/issues/8961" class="external">https://bugs.ruby-lang.org/issues/8961</a></p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=421052013-09-30T22:40:05Znaruse (Yui NARUSE)naruse@airemix.jp
<ul><li><strong>Target version</strong> set to <i>Ruby 2.1.0</i></li></ul> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=421112013-09-30T23:53:19Zavdi (Avdi Grimm)avdi@avdi.codes
<ul></ul><p>On Fri, Sep 27, 2013 at 6:59 AM, headius (Charles Nutter) <<br>
<a href="mailto:headius@headius.com" class="email">headius@headius.com</a>> wrote:</p>
<blockquote>
<p>I propose adding MutexedDelegator as a simple way to wrap any object with<br>
a thread-safe wrapper, via existing delegation logic in delegate.rb.</p>
</blockquote>
<p>I think that's a wonderful idea. What do you think of the name<br>
SynchronizedDelegator?</p>
<p>--<br>
Avdi Grimm<br>
<a href="http://avdi.org" class="external">http://avdi.org</a></p>
<p>I only check email twice a day. to reach me sooner, go to<br>
<a href="http://awayfind.com/avdi" class="external">http://awayfind.com/avdi</a></p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=421152013-10-01T06:13:52Zheadius (Charles Nutter)headius@headius.com
<ul></ul><p>SynchronizedDelegator is a better name, and the code should use Monitor instead of Mutex so it can be reentrant. I'll do that now.</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=421162013-10-01T07:23:13Zavdi (Avdi Grimm)avdi@avdi.codes
<ul></ul><p>On Mon, Sep 30, 2013 at 5:13 PM, headius (Charles Nutter) <<br>
<a href="mailto:headius@headius.com" class="email">headius@headius.com</a>> wrote:</p>
<blockquote>
<p>and the code should use Monitor instead of Mutex so it can be reentrant.</p>
</blockquote>
<p>I'm trying to think of a case in which this would matter. Since it's<br>
wrapping another object, we don't have to worry about the case where a<br>
synced method calls another synced method on self.</p>
<p>Hmmm... I guess there's the case where synced_obj.foo receives a block, and<br>
someone calls synced_obj.bar within that block.</p>
<p>I only bring this up because Monitor introduces a (small?) performance hit<br>
over Mutex.</p>
<p>--<br>
Avdi Grimm<br>
<a href="http://avdi.org" class="external">http://avdi.org</a></p>
<p>I only check email twice a day. to reach me sooner, go to<br>
<a href="http://awayfind.com/avdi" class="external">http://awayfind.com/avdi</a></p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=421172013-10-01T08:02:55Zheadius (Charles Nutter)headius@headius.com
<ul></ul><p>I implemented this and a simple test in <a href="https://github.com/ruby/ruby/pull/405" class="external">https://github.com/ruby/ruby/pull/405</a></p>
<p>If approved, I can merge that or commit to trunk directly.</p>
<p>The performance impact of Monitor is a separate issue; Monitor should probably be implemented natively to get maximum performance. I'm considering doing that for JRuby as well. As you point out, reentrancy is needed for any code that might call out to a block which might call back in.</p>
<p>There's not a great deal we can do to speed up Monitor as it is currently written, but perhaps you could file a bug for that and we can see about improving things.</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=421332013-10-01T14:11:27Znobu (Nobuyoshi Nakada)nobu@ruby-lang.org
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/42133/diff?detail_id=30567">diff</a>)</li></ul> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=421362013-10-01T16:55:38Znaruse (Yui NARUSE)naruse@airemix.jp
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li><li><strong>Assignee</strong> set to <i>ko1 (Koichi Sasada)</i></li></ul><p>ko1 will write objection.</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=421732013-10-02T03:12:52Zheadius (Charles Nutter)headius@headius.com
<ul></ul><p>naruse (Yui NARUSE) wrote:</p>
<blockquote>
<p>ko1 will write objection.</p>
</blockquote>
<p>I look forward to reading that objection :-)</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=425482013-10-22T21:23:30Zheadius (Charles Nutter)headius@headius.com
<ul></ul><p>Still waiting to read ko1's objection. I am prepared to commit a monitor-based delegator if we go forward.</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=425502013-10-22T23:55:47Zko1 (Koichi Sasada)
<ul></ul><p>Sorry for late.</p>
<hr>
<p>Summary: I believe we need more experience before including this library as standard.</p>
<p>(1) Try gem first</p>
<p>Basically, libraries written in Ruby can be provided by a gem easilly.<br>
We can prove the usefulness with real experiece by this approach.<br>
In other words, we shouldn't provide any libraries without such proof.</p>
<p>(2) Misleading design</p>
<p>I'm afraid that this library introduce bugs under misunderstanding.</p>
<p>For example, people consider that this object is without worry about<br>
synchronization, people may write the following program.</p>
<pre><code># In fact, *I* wrote this program first without any question!!
####
require 'delegate'
require 'monitor'
class SynchronizedDelegator < SimpleDelegator
def initialize(*)
super
@monitor = Monitor.new
end
def method_missing(m, *args, &block)
begin
monitor = @monitor
monitor.mon_enter
super
ensure
monitor.mon_exit
end
end
end
sdel_ary = SynchronizedDelegator.new([0])
ary = [0]
m = Mutex.new
ts = (1..2).map{
Thread.new{
100_000.times{
sdel_ary[0] += 1 # -> 1
sdel_ary[0] -= 1 # -> 0
m.synchronize{
ary[0] += 1
ary[0] -= 1
}
}
}
}
ts.each{|t| t.join}
p sdel_ary #=> [40] # or something wrong result
p ary #=> [0]
####
</code></pre>
<p>At first I see this result, I can't understand why.<br>
Of course, this program is completely bad program.<br>
It is completely my mistake.</p>
<p>But I think this design will lead such misunderstanding and bugs easily.</p>
<p>To avoid a such bug, I define the inc() and sub() method in Array.</p>
<pre><code>####
class Array
def inc; self[0] += 1; end
def sub; self[0] -= 1; end
end
sdel_ary = SynchronizedDelegator.new([0])
ts = (1..2).map{
Thread.new{
100_000.times{
sdel_ary.inc
sdel_ary.sub
}
}
}
ts.each{|t| t.join}
p sdel_ary[0] #=> 200000
####
</code></pre>
<p>This works completely.</p>
<p>But a person who assumes sdel_ary is free from consideration about locking,<br>
can write the following program:</p>
<pre><code>####
class << sdel_ary
def inc; self[0] += 1; end
def sub; self[0] -= 1; end
end
ts = (1..2).map{
Thread.new{
100_000.times{
sdel_ary.inc
sdel_ary.sub
}
}
}
ts.each{|t| t.join}
p sdel_ary[0] #=> 229
####
</code></pre>
<p>This doesn't work correctly (different from the person expect).</p>
<p>I feel we can find other cases.</p>
<p>Maybe professional programmers about concurrency program can avoid such silly bugs.<br>
But if we introduce it as standard library, I'm afraid they are not.</p>
<p>(3) Lock based thraed programming</p>
<p>This is my opinion. So it is weak objection for this proposal.</p>
<p>I believe lock based thread programming introduced many bugs.<br>
(Synchronized) Queue or more high-level structures should be used.</p>
<p>Or use Mutex (or monitor) explicitly for fing-grain locking.<br>
It bothers programmers, so programmers use other approachs such as Queue (I hope).</p>
<p>Summary:</p>
<p>Mainly, my objection is based on (1) and (2).<br>
Concurrency is a very difficult theme.<br>
I feel 2.1 is too early to include this feature.<br>
At least, we need more experience about this feature to introduce.</p>
<p>I'm not against that professionals use this libarary.</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=425522013-10-23T04:07:42Zheadius (Charles Nutter)headius@headius.com
<ul></ul><p>ko1 (Koichi Sasada) wrote:</p>
<blockquote>
<p>(1) Try gem first</p>
</blockquote>
<p>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.</p>
<p>So the rest of this may be moot, but I'll proceed anyway.</p>
<blockquote>
<p>(2) Misleading design</p>
<p>I'm afraid that this library introduce bugs under misunderstanding.</p>
<p>For example, people consider that this object is without worry about<br>
synchronization, people may write the following program.</p>
</blockquote>
<p>Someone else raised a concern about += and friends, but there's <em>no</em> way in a library to ever make those operations thread safe (actually, atomic). That's what the "atomic" gem provides.</p>
<p>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.</p>
<p>FWIW, Java's ++ and -- and += and friends are <em>also</em> not atomic.</p>
<p>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.</p>
<blockquote>
<p>At first I see this result, I can't understand why.<br>
Of course, this program is completely bad program.<br>
It is completely my mistake.</p>
<p>But I think this design will lead such misunderstanding and bugs easily.</p>
</blockquote>
<p>But this is not possible to fix in current Ruby and all other languages I know don't guarantee any atomicity here either.</p>
<blockquote>
<p>To avoid a such bug, I define the inc() and sub() method in Array.</p>
</blockquote>
<p>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.</p>
<blockquote>
<p>This works completely.</p>
<p>But a person who assumes sdel_ary is free from consideration about locking,<br>
can write the following program:</p>
</blockquote>
<p>This is perhaps a valid concern. SynchronizedDelegate could use a method_added hook to wrap new methods, however. Is it warranted?</p>
<pre><code> 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
</code></pre>
<p>Or something like that.</p>
<blockquote>
<p>Maybe professional about concurrency program can avoid such silly bugs.<br>
But if we introduce it as standard library, I'm afraid they are not.</p>
</blockquote>
<p>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.</p>
<blockquote>
<p>(3) Lock based thraed programming</p>
<p>This is my opinion. So it is weak objection for this proposal.</p>
<p>I believe lock based thread programming introduced many bugs.<br>
(Synchronized) Queue or more high-level structures should be used.</p>
<p>Or use Mutex (or monitor) explicitly for fing-grain locking.<br>
It bothers programmers, so programmers use other approachs such as Queue (I hope).</p>
</blockquote>
<p>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.</p>
<p>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.</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=628772017-02-06T03:04:34Zko1 (Koichi Sasada)
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/62877/diff?detail_id=43896">diff</a>)</li></ul> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=628782017-02-06T03:17:19Zko1 (Koichi Sasada)
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Rejected</i></li></ul><p>Sorry for long absence.</p>
<blockquote>
<p>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.</p>
</blockquote>
<p>inc/dec is only an example.</p>
<blockquote>
<p>But this is not possible to fix in current Ruby and all other languages I know don't guarantee any atomicity here either.</p>
</blockquote>
<p>I believe ruby should move to non-sharing way. Performance on this approach is not so good, but enough I believe.</p> Ruby master - Feature #8556: MutexedDelegator as a trivial way to make an object thread-safehttps://bugs.ruby-lang.org/issues/8556?journal_id=955122021-12-23T23:40:08Zhsbt (Hiroshi SHIBATA)hsbt@ruby-lang.org
<ul><li><strong>Project</strong> changed from <i>14</i> to <i>Ruby master</i></li><li><strong>Target version</strong> deleted (<del><i>Ruby 2.1.0</i></del>)</li></ul>