Re: [PATCH 1/5] MSVC: Windows-native implementation for subset of Pthreads API

On Fri, Jan 8, 2010 at 4:32 AM, Dmitry Potapov [off-list ref] wrote:

On Thu, Jan 07, 2010 at 10:54:57PM +0100, Johannes Sixt wrote:

quoted

+
+int pthread_cond_wait(pthread_cond_t *cond, CRITICAL_SECTION *mutex)
+{
+     /* serialize access to waiters count */
+     EnterCriticalSection(&cond->waiters_lock);
+     ++cond->waiters;
+     LeaveCriticalSection(&cond->waiters_lock);

InterlockedIncrement(&cond->waiters);

quoted

+
+     /*
+      * Unlock external mutex and wait for signal.
+      * NOTE: we've held mutex locked long enough to increment
+      * waiters count above, so there's no problem with
+      * leaving mutex unlocked before we wait on semaphore.
+      */
+     LeaveCriticalSection(mutex);
+
+     /* let's wait - ignore return value */
+     WaitForSingleObject(cond->sema, INFINITE);
+
+     /* we're done waiting, so make sure we decrease waiters count */
+     EnterCriticalSection(&cond->waiters_lock);
+     --cond->waiters;
+     LeaveCriticalSection(&cond->waiters_lock);

InterlockedDecrement(&cond->waiters);

Nice.

quoted

+
+     /* lock external mutex again */
+     EnterCriticalSection(mutex);
+
+     return 0;
+}
+
+int pthread_cond_signal(pthread_cond_t *cond)
+{
+     int have_waiters;
+
+     /* serialize access to waiters count */
+     EnterCriticalSection(&cond->waiters_lock);
+     have_waiters = cond->waiters > 0;
+     LeaveCriticalSection(&cond->waiters_lock);

AFAIK, Win32 API assumes that reading LONG is always atomic, so
the critical section is not really necesary here, but you need
to declare 'waiters' as 'volatile':

"Simple reads and writes to properly-aligned 32-bit variables are
atomic operations."
http://msdn.microsoft.com/en-us/library/ms684122(VS.85).aspx

In other words: Yes, you are right.

quoted

+
+int pthread_cond_init(pthread_cond_t *cond, const void *unused)
+{
+     cond->waiters = 0;
+
+     InitializeCriticalSection(&cond->waiters_lock);

Is waiters_lock really necessary?

(Yeah, I moved this one to the end)

No, I think you've proven that it isn't.

-- 
Erik "kusma" Faye-Lund