Richard Oudkerk <shibt...@gmail.com> added the comment:
Py_LOCAL_INLINE(int)
_PyCOND_WAIT_MS(PyCOND_T *cv, PyMUTEX_T *cs, DWORD ms)
{
DWORD wait;
cv->waiting++;
PyMUTEX_UNLOCK(cs);
/* "lost wakeup bug" would occur if the caller were interrupted here,
* but we are safe because we are using a semaphore wich has an internal
* count.
*/
wait = WaitForSingleObject(cv->sem, ms);
PyMUTEX_LOCK(cs);
if (wait != WAIT_OBJECT_0)
--cv->waiting;
/* Here we have a benign race condition with PyCOND_SIGNAL.
* When failure occurs or timeout, it is possible that
* 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.
*/
...
Are you really sure this race is benign?
If cv->waiting gets double decremented then it can become negative.
PyCOND_SIGNAL() is defined as
Py_LOCAL_INLINE(int)
PyCOND_SIGNAL(PyCOND_T *cv)
{
if (cv->waiting) {
cv->waiting--;
return ReleaseSemaphore(cv->sem, 1, NULL) ? 0 : -1;
}
return 0;
}
While cv->waiting is negative, each call of PyCOND_SIGNAL() decrements
cv->waiting, and increments the semaphore, while each call of PyCOND_WAIT()
will increment cv->waiting and decrement the semaphore.
So if calls of PyCOND_SIGNAL() outnumber calls of PyCOND_WAIT() then we can
have cv->waiting becoming very negative and the semaphore overflowing.
Maybe just changing the test in PyCOND_SIGNAL() to
if (cv->waiting > 0) {
would be enough, but I am not convinced.
----------
nosy: +sbt
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue15038>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
