Bug #5195

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

Added by Glass_saga (Masaki Matsushita) 9 months ago. Updated 9 months ago.

[ruby-dev:44400]
Status:Closed Start date:08/16/2011
Priority:Normal Due date:
Assignee:kosaki (Motohiro KOSAKI) % Done:

100%
Category:lib
Target version:2.0.0
ruby -v:ruby 1.9.4dev (2011-08-14 trunk 32972) [x86_64-linux]

Description

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

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 `<main>'

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をパスします。

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

Related issues

related to ruby-trunk - Bug #5258: SizedQueueにBug #5195と同様のバグ Assigned 09/01/2011
related to ruby-trunk - Bug #5355: Sync_mにBug #5195やBug #5258と同様のバグ Assigned 09/23/2011

Associated revisions

Revision 33121
Added by kosaki (Motohiro KOSAKI) 9 months ago

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

History

[ruby-dev:44401]#1
Updated by kosaki (Motohiro KOSAKI) 9 months ago

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

#2
Updated by kosaki (Motohiro KOSAKI) 9 months 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]

[ruby-dev:44439]#3
Updated by kosaki (Motohiro KOSAKI) 9 months ago

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

Also available in: Atom PDF