Project

General

Profile

Actions

Bug #5195

closed

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

Added by Glass_saga (Masaki Matsushita) over 12 years ago. Updated over 12 years ago.

Status:
Closed
Target version:
ruby -v:
ruby 1.9.4dev (2011-08-14 trunk 32972) [x86_64-linux]
Backport:
[ruby-dev:44400]

Description

=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


Files

patch.diff (432 Bytes) patch.diff Glass_saga (Masaki Matsushita), 08/16/2011 10:18 PM

Related issues 2 (0 open2 closed)

Related to Ruby master - Bug #5258: SizedQueueにBug #5195と同様のバグClosedkosaki (Motohiro KOSAKI)09/01/2011Actions
Related to Ruby master - Bug #5355: Sync_mにBug #5195やBug #5258と同様のバグClosedkosaki (Motohiro KOSAKI)09/23/2011Actions

Updated by kosaki (Motohiro KOSAKI) over 12 years ago

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

Updated by kosaki (Motohiro KOSAKI) over 12 years ago

  • 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]

Updated by kosaki (Motohiro KOSAKI) over 12 years ago

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

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0