Message163128
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. |
|
Date |
User |
Action |
Args |
2012-06-18 23:20:35 | sbt | set | recipients:
+ sbt, loewis, paul.moore, pitrou, kristjan.jonsson, vstinner, python-dev |
2012-06-18 23:20:35 | sbt | set | messageid: <1340061635.54.0.0801467862062.issue15038@psf.upfronthosting.co.za> |
2012-06-18 23:20:34 | sbt | link | issue15038 messages |
2012-06-18 23:20:34 | sbt | create | |
|