Bug #5195: Queue#popでsleepしているthreadをwakeupさせるとQueueの@waitingにそのthreadがpushされてしまう

• Status: Closed
• Assignee: kosaki (Motohiro KOSAKI)
• Target version: 2.0.0
• ruby -v: ruby 1.9.4dev (2011-08-14 trunk 32972) [x86_64-linux]

[Glass_saga (Masaki Matsushita)]

=begin
次のようなコードを実行すると、

require 'thread'

queue = Queue.new

t1 = Thread.start { queue.pop; p 1 }

nil until t1.stop?
t1.wakeup
nil until t1.stop?

t2 = Thread.start { queue.pop; p 2 }

nil until t1.stop? && t2.stop?

p t1, t2
queue.instance_eval{ p @waiting }

2.times{ queue.push(nil) }

t1.join
t2.join

以下のような結果となります。

#<Thread:0x000000014d8108 sleep>
#<Thread:0x000000014d8090 sleep>
[#<Thread:0x000000014d8108 sleep>, #<Thread:0x000000014d8108 sleep>, #<Thread:0x000000014d8090 sleep>]
1
/home/masaki/ruby/queue_waiting.rb:22:in join': deadlock detected (fatal) from /home/masaki/ruby/queue_waiting.rb:22:in '

lib/thread.rbの184行目でQueue#popが以下のように定義されていますが、

def pop(non_block=false)
@mutex.synchronize{
while true
if @que.empty?
raise ThreadError, "queue empty" if non_block
@waiting.push Thread.current
@mutex.sleep
else
return @que.shift
end
end
}
end

このコードではQueue#popでsleepしているthreadをwakeupさせると、既にpush済みのThread.currentが再度@waitingにpushされてしまいます。
これはバグではないでしょうか。

patchを添付します。
このpatchでは遅くなってしまうのではないかと懸念しましたが、

require 'benchmark'
require 'thread'

def push_pop
q = Queue.new
time = 100000

push = Thread.start do
time.times { q.push(nil) }
end

pop = Thread.start do
time.times { q.pop }
end

push.join
pop.join
end

Benchmark.bm do |x|
x.report("before") { push_pop }
end

というベンチマークを作って試してみたところ、
patchの適用以前が

user system total real
0.460000 0.000000 0.460000 ( 0.452595)
0.500000 0.040000 0.540000 ( 0.529834)
0.410000 0.030000 0.440000 ( 0.433928)

適用後が

user system total real
0.480000 0.060000 0.540000 ( 0.505939)
0.450000 0.000000 0.450000 ( 0.449451)
0.430000 0.010000 0.440000 ( 0.435586)

となったのでそれほどには遅くなっていないようです。
patchの適用後もtest/thread/test_queue.rbをパスします。

=end

[kosaki (Motohiro KOSAKI)]

• Status changed from Open to Assigned
• Assignee set to kosaki (Motohiro KOSAKI)

[kosaki (Motohiro KOSAKI)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r33121.
Masaki, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

• lib/thread.rb (Queue#pop): fix a race against Thread.wakeup.
  Patch by Masaki Matsushita <glass.saga at gmail dot com>
  [Bug #5195] [ruby-dev:44400]

[kosaki (Motohiro KOSAKI)]

コミットしましたが、１）あからさまに場当たり的workaroundであること ２）作為的なテストケースであること により1.9.3にbackportはしませんでした。