Re: [PATCH/RFC] mingw: implement PTHREAD_MUTEX_INITIALIZER

On Tue, Oct 25, 2011 at 5:28 PM, Johannes Sixt [off-list ref] wrote:

Am 10/25/2011 16:55, schrieb Erik Faye-Lund:

quoted

+int pthread_mutex_lock(pthread_mutex_t *mutex)
+{
+     if (mutex->autoinit) {
+             if (InterlockedCompareExchange(&mutex->autoinit, -1, 1) != -1) {
+                     pthread_mutex_init(mutex, NULL);
+                     mutex->autoinit = 0;
+             } else
+                     while (mutex->autoinit != 0)
+                             ; /* wait for other thread */
+     }

The double-checked locking idiom. Very suspicious. Can you explain why it
works in this case? Why are no Interlocked functions needed for the other
accesses of autoinit? ("It is volatile" is the wrong answer to this last
question, BTW.)

I agree that it should look a bit suspicious; I'm generally skeptical
whenever I see 'volatile' in threading-code myself. But I think it's
the right answer in this case. "volatile" means that the compiler
cannot optimize away accesses, which is sufficient in this case.

Basically, the thread that gets the original 1 returned from
InterlockedCompareExchange is the only one who writes to
mutex->autoinit. All other threads only read the value, and the
volatile should make sure they actually do. Since all 32-bit reads and
writes are atomic on Windows (see
http://msdn.microsoft.com/en-us/library/windows/desktop/ms684122(v=vs.85).aspx
"Simple reads and writes to properly-aligned 32-bit variables are
atomic operations.") and mutex->autoinit is a LONG, this should be
safe AFAICT. In fact, Windows specifically does not have any
explicitly atomic writes exactly for this reason.

The only ways mutex->autoinit can be updated is:
- InterlockedCompareExchange compares it to 1, finds it's identical
and inserts -1
- intialization is done
Both these updates happens from the same thread.

Yes, details like this should probably go into the commit message ;)

If there's something I'm missing, please let me know.