From: Johannes Sixt <j.sixt@viscovery.net>
Date: Tue, 08 Jun 2010 18:46:17 GMT
Subject: Re: [RFT PATCH 2/2] win32: optimize pthread_cond_broadcast
Message-ID: <4C0E8FF9.7020500@viscovery.net>
In-Reply-To: <4C0E71B2.1060904@gnu.org>

Am 08.06.2010 18:37, schrieb Paolo Bonzini:
> On 06/08/2010 06:30 PM, Johannes Sixt wrote:
>> Am 07.06.2010 15:38, schrieb Paolo Bonzini:
>>> @@ -172,9 +172,10 @@ int pthread_cond_broadcast(pthread_cond_t *cond)
>>> * As in pthread_cond_signal, access to cond->waiters and
>>> * cond->was_broadcast is locked via the external mutex.
>>> */
>>> -
>>> - if ((cond->was_broadcast = cond->waiters> 0)) {
>>> + if (cond->waiters> 0) {
>>> BOOLEAN result;
>>> + cond->was_broadcast = cond->waiters> 1;
>>> +
>>
>> It is possible that you set was_broadcast to 1 here, while another
>> thread still sees was_broadcast == 0 in cond_wait.
>
> That still cannot happen, because pthread_cond_wait will be locked on
> the semaphore until the ReleaseSemaphore. The only race that exists is
> between broadcast/signal's ReleaseSemaphore and wait's
> WaitForSingleObject. This is benign, and exists before my patch. But in
> all cases the code before ReleaseSemaphore is serialized WRT to the code
> after wait's WaitForSingleObject.

I think I've stared at the code long enough now to see that you are right. 
All counterexamples that I thought I could make up to disprove you didn't 
do it :-)

-- Hannes