Author sbt
Recipients brian.curtin, kristjan.jonsson, loewis, pitrou, sbt, tim.golden
Date 2011-03-21.16:34:58
SpamBayes Score 8.82928e-09
Marked as misclassified No
Message-id <1300725299.23.0.901625236244.issue11618@psf.upfronthosting.co.za>
In-reply-to
Content
> Btw, the locktimeout.patch appears to have a race condition.  
> LeaveNonRecursiveMutex may SetEvent when there is no thread waiting 
> (because a timeout just occurred, but the thread on which it happened 
> is still somewhere around line #62 ).  This will cause the next 
> WaitForSingleObject() to succeed, when it shouldn't.

I believe the lock is still in a consistent state.  If this race happens and SetEvent() is called then we will must have mutex->owned > -1 because the timed out waiter is still counted by mutex->owned.  This prevents the tests involving interlocked functions from giving true.  Thus WaitForSingleObject() is the ONLY way for a waiter to get the lock.

In other words, as soon as a timeout happens the fast "interlocked path" gets blocked.  It is only unblocked again after a call to WaitForSingleObject() succeeds: then the thread which now owns the lock fixes mutex->owned using mutex->timeouts and the interlocked path is operational again (unless another timeout happens).

I can certainly understand the desire to follow the KISS principle.
History
Date User Action Args
2011-03-21 16:34:59sbtsetrecipients: + sbt, loewis, pitrou, kristjan.jonsson, tim.golden, brian.curtin
2011-03-21 16:34:59sbtsetmessageid: <1300725299.23.0.901625236244.issue11618@psf.upfronthosting.co.za>
2011-03-21 16:34:58sbtlinkissue11618 messages
2011-03-21 16:34:58sbtcreate