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

From: Johannes Sixt <j6t@kdbg.org>
Date: Jan 12, 2010, 21:13 UTC
Message-ID: <201001122213.38287.j6t@kdbg.org>
In-Reply-To: <40aa078e1001080258n67e0711sf4733a99d512bf1@mail.gmail.com>

On Freitag, 8. Januar 2010, Erik Faye-Lund wrote:

Show 8 quoted lines

> 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