Don't decrement below zero. And add more tests.

diff --git a/Lib/Queue.py b/Lib/Queue.py
index ad65cf0b579b63158b192a9b7aa3e40defa8553f..51ad354260874e593d8ed25c0fc6a30711306aa0 100644 (file)
--- a/Lib/Queue.py
+++ b/Lib/Queue.py
@@ -56,11 +56,12 @@ class Queue:
         """
         self.all_tasks_done.acquire()
         try:
-            self.unfinished_tasks = unfinished = self.unfinished_tasks - 1
+            unfinished = self.unfinished_tasks - 1
             if unfinished <= 0:
                 if unfinished < 0:
                     raise ValueError('task_done() called too many times')
                 self.all_tasks_done.notifyAll()
+            self.unfinished_tasks = unfinished
         finally:
             self.all_tasks_done.release()
 

diff --git a/Lib/test/test_queue.py b/Lib/test/test_queue.py
index 77a1c9da118a265519b14606e549fd09395bac64..b3f24dff5177a1b3c1906f223e9b9c48535ce427 100644 (file)
--- a/Lib/test/test_queue.py
+++ b/Lib/test/test_queue.py
@@ -228,6 +228,9 @@ def worker(q):
     global cum
     while True:
         x = q.get()
+        if x is None:
+            q.task_done()
+            return
         cumlock.acquire()
         try:
             cum += x
@@ -239,18 +242,29 @@ def QueueJoinTest(q):
     global cum
     cum = 0
     for i in (0,1):
-        t = threading.Thread(target=worker, args=(q,))
-        t.setDaemon(True)
-        t.start()
+        threading.Thread(target=worker, args=(q,)).start()
     for i in xrange(100):
         q.put(i)
     q.join()
     verify(cum==sum(range(100)), "q.join() did not block until all tasks were done")
+    for i in (0,1):
+        q.put(None)         # instruct the threads to close
+    q.join()                # verify that you can join twice
+
+def QueueTaskDoneTest(q)
+    try:
+        q.task_done()
+    except ValueError:
+        pass
+    else:
+        raise TestFailed("Did not detect task count going negative")
 
 def test():
     q = Queue.Queue()
+    QueueTaskDoneTest(q)
     QueueJoinTest(q)
     QueueJoinTest(q)
+    QueueTaskDoneTest(q)
 
     q = Queue.Queue(QUEUE_SIZE)
     # Do it a couple of times on the same queue