[Python-checkins] cpython: Issue #15038:

[kristjan.jonsson]

http://hg.python.org/cpython/rev/110b38c36a31
changeset:   77515:110b38c36a31
user:        Kristjan Valur Jonsson <sweskman at gmail.com>
date:        Tue Jun 19 10:10:09 2012 +0000
summary:
  Issue #15038:
Fix incorrect test of the condition variable state, spotted by
Richard Oudkerk.  This could cause the internal condition variable
to grow without bounds.

files:
  Python/condvar.h |  18 +++++++++++++-----
  1 files changed, 13 insertions(+), 5 deletions(-)


diff --git a/Python/condvar.h b/Python/condvar.h
--- a/Python/condvar.h
+++ b/Python/condvar.h
@@ -177,7 +177,7 @@
 typedef struct _PyCOND_T
 {
     HANDLE sem;
-    int waiting;
+    int waiting; /* to allow PyCOND_SIGNAL to be a no-op */
 } PyCOND_T;
 
 Py_LOCAL_INLINE(int)
@@ -222,6 +222,10 @@
          * PyCOND_SIGNAL also decrements this value
          * and signals releases the mutex.  This is benign because it
          * just means an extra spurious wakeup for a waiting thread.
+         * ('waiting' corresponds to the semaphore's "negative" count and
+         * we may end up with e.g. (waiting == -1 && sem.count == 1).  When
+         * a new thread comes along, it will pass right throuhgh, having
+         * adjusted it to (waiting == 0 && sem.count == 0).
          */
          
     if (wait == WAIT_FAILED)
@@ -246,10 +250,14 @@
 Py_LOCAL_INLINE(int)
 PyCOND_SIGNAL(PyCOND_T *cv)
 {
-    if (cv->waiting) {
+    /* this test allows PyCOND_SIGNAL to be a no-op unless required
+     * to wake someone up, thus preventing an unbounded increase of
+     * the semaphore's internal counter.
+     */
+    if (cv->waiting > 0) {
         /* notifying thread decreases the cv->waiting count so that
-         * a delay between notify and wakeup doesn't cause a number
-         * of extra ReleaseSemaphore calls
+         * a delay between notify and actual wakeup of the target thread
+         * doesn't cause a number of extra ReleaseSemaphore calls.
          */
         cv->waiting--;
         return ReleaseSemaphore(cv->sem, 1, NULL) ? 0 : -1;
@@ -260,7 +268,7 @@
 Py_LOCAL_INLINE(int)
 PyCOND_BROADCAST(PyCOND_T *cv)
 {
-    if (cv->waiting) {
+    if (cv->waiting > 0) {
         return ReleaseSemaphore(cv->sem, cv->waiting, NULL) ? 0 : -1;
 		cv->waiting = 0;
     }

-- 
Repository URL: http://hg.python.org/cpython