Subject: Re: [PATCH] MSVC: port pthread code to native Windows threads

From: Paolo Bonzini <[email-masked]>
Date: Wed, 04 Nov 2009 12:23:15 +0100

The condition variable implementation seems more complicated than 
necessary.  The mutex can be used to protect access to cond->waiters, so 
waiters_lock is not necessary.  On the other hand, it seems to me that 
pthread_cond_signal should be the one that decrements the waiters count. 
  Otherwise, a loop like

	while (pthread_cond_signal (cond, mutex));

will fill the semaphore with signals and the waiters will get lots of 
spurious accesses.

static __inline int pthread_cond_wait(pthread_cond_t *cond,
                                       CRITICAL_SECTION *mutex)
{
	int ret = 0;

	/* the mutex protects access to waiters count */
	++cond->waiters;

	/*
	 * Unlock external mutex and wait for signal.
	 * NOTE: cond->waiters > 0 now.  If pthread_cond_signal
	 * is called after leaving mutex unlocked before we wait on
	 * semaphore, it will add a signal to the semaphore,
	 * and we'll happily go on with the wait.  This would not
	 * happen with an event, for example.
	 */
	LeaveCriticalSection(mutex);
	if (0 != WaitForSingleObject(cond->sema, INFINITE))
		ret = -1;

	EnterCriticalSection(mutex);
	return ret;
}

static __inline int pthread_cond_signal(pthread_cond_t *cond)
{
	/* the mutex protects access to waiters count */
	if (cond->waiters > 0) {
		--cond->waiters;
		return ReleaseSemaphore(cond->sema, 1, NULL) ? 0 : -1;
	} else
		return 0;
}

Paolo