From: Johannes Sixt <j6t@kdbg.org>
Date: Tue, 12 Jan 2010 21:13:38 GMT
Subject: Re: [PATCH 1/5] MSVC: Windows-native implementation for subset of Pthreads API
Message-ID: <201001122213.38287.j6t@kdbg.org>
In-Reply-To: <40aa078e1001080258n67e0711sf4733a99d512bf1@mail.gmail.com>

On Freitag, 8. Januar 2010, Erik Faye-Lund wrote:
> On Fri, Jan 8, 2010 at 4:32 AM, Dmitry Potapov <dpotapov@gmail.com> wrote:
> > 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

But then the next sentence is:

"However, access is not guaranteed to be synchronized. If two threads are 
reading and writing from the same variable, you cannot determine if one 
thread will perform its read operation before the other performs its write 
operation."

This goes without saying, IOW, those Microsofties don't know what they write, 
which makes the documentation a bit less trustworthy.

Nevertheless, I rewrote the code to use Interlocked* functions, and then read 
the documentation again. InterlockedIncrement reads, for example:

"... This function is atomic with respect to calls to other interlocked 
functions."

In particular, it doesn't say that it is atomic WRT reads such as we have 
here:

> >> +     /* we're done waiting, so make sure we decrease waiters count */
> >> +     EnterCriticalSection(&cond->waiters_lock);
> >> +     --cond->waiters;
> >> +     LeaveCriticalSection(&cond->waiters_lock);

I've no assembly-fu, but I could imagine that it does not matter, but I really 
would have confirmation from an x86 guru.

-- Hannes