kosaki	2012-09-09 21:32:33 +0900 (Sun, 09 Sep 2012)

  New Revision: 36938

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=36938

  Log:
    * lib/thread.rb (Queue#pop): Fixed double registration issue when
      mutex.sleep is interrupted. [Bug #5258] [ruby-dev:44448]
    * lib/thread.rb (SizedQueue#push): ditto.
    
    * test/thread/test_queue.rb (test_sized_queue_and_wakeup,
    test_queue_pop_interrupt, test_sized_queue_pop_interrupt,
    test_sized_queue_push_interrupt): new tests.

  Modified files:
    trunk/ChangeLog
    trunk/lib/thread.rb
    trunk/test/thread/test_queue.rb

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 36937)
+++ ChangeLog	(revision 36938)
@@ -1,3 +1,13 @@
+Sun Sep  9 21:21:15 2012  KOSAKI Motohiro  <kosaki.motohiro@g...>
+
+	* lib/thread.rb (Queue#pop): Fixed double registration issue when
+	  mutex.sleep is interrupted. [Bug #5258] [ruby-dev:44448]
+	* lib/thread.rb (SizedQueue#push): ditto.
+
+	* test/thread/test_queue.rb (test_sized_queue_and_wakeup,
+	test_queue_pop_interrupt, test_sized_queue_pop_interrupt,
+	test_sized_queue_push_interrupt): new tests.
+
 Sun Sep  9 20:20:31 2012  KOSAKI Motohiro  <kosaki.motohiro@g...>
 
 	* lib/sync.rb (Sync_m#sync_lock): Fixed wakeup/raise unsafe code.
Index: lib/thread.rb
===================================================================
--- lib/thread.rb	(revision 36937)
+++ lib/thread.rb	(revision 36938)
@@ -182,16 +182,20 @@
   #
   def pop(non_block=false)
     @mutex.synchronize{
-      while true
-        if @que.empty?
-          raise ThreadError, "queue empty" if non_block
-          # @waiting.include? check is necessary for avoiding a race against
-          # Thread.wakeup [Bug 5195]
-          @waiting.push Thread.current unless @waiting.include?(Thread.current)
-          @mutex.sleep
-        else
-          return @que.shift
+      begin
+        while true
+          if @que.empty?
+            raise ThreadError, "queue empty" if non_block
+            # @waiting.include? check is necessary for avoiding a race against
+            # Thread.wakeup [Bug 5195]
+            @waiting.push Thread.current unless @waiting.include?(Thread.current)
+            @mutex.sleep
+          else
+            return @que.shift
+          end
         end
+      ensure
+        @waiting.delete(Thread.current)
       end
     }
   end
@@ -298,10 +302,14 @@
   #
   def push(obj)
     @mutex.synchronize{
-      while true
-        break if @que.length < @max
-        @queue_wait.push Thread.current
-        @mutex.sleep
+      begin
+        while true
+          break if @que.length < @max
+          @queue_wait.push Thread.current unless @queue_wait.include?(Thread.current)
+          @mutex.sleep
+        end
+      ensure
+        @queue_wait.delete(Thread.current)
       end
 
       @que.push obj
Index: test/thread/test_queue.rb
===================================================================
--- test/thread/test_queue.rb	(revision 36937)
+++ test/thread/test_queue.rb	(revision 36938)
@@ -56,6 +56,48 @@
     assert_equal(1, q.max)
   end
 
+  def test_sized_queue_and_wakeup
+    sq = SizedQueue.new(1)
+    sq.push(0)
+
+    t1 = Thread.start { sq.push(1) ; sleep }
+
+    sleep 0.1 until t1.stop?
+    t1.wakeup
+    sleep 0.1 until t1.stop?
+
+    t2 = Thread.start { sq.push(2) }
+    sleep 0.1 until t1.stop? && t2.stop?
+
+    queue_wait = sq.instance_eval{ @queue_wait }
+    assert_equal(queue_wait.uniq, queue_wait)
+  end
+
+  def test_queue_pop_interrupt
+    q = Queue.new
+    t1 = Thread.new { q.pop }
+    sleep 0.01 until t1.stop?
+    t1.kill.join
+    assert_equal(0, q.num_waiting)
+  end
+
+  def test_sized_queue_pop_interrupt
+    q = SizedQueue.new(1)
+    t1 = Thread.new { q.pop }
+    sleep 0.01 until t1.stop?
+    t1.kill.join
+    assert_equal(0, q.num_waiting)
+  end
+
+  def test_sized_queue_push_interrupt
+    q = SizedQueue.new(1)
+    q.push(1)
+    t1 = Thread.new { q.push(2) }
+    sleep 0.01 until t1.stop?
+    t1.kill.join
+    assert_equal(0, q.num_waiting)
+  end
+
   def test_thr_kill
     bug5343 = '[ruby-core:39634]'
     Dir.mktmpdir {|d|

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/