From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Wed, 26 Oct 2011 13:08:19 GMT
Subject: Re: [msysGit] Re: [PATCH/RFC] mingw: implement PTHREAD_MUTEX_INITIALIZER
Message-ID: <CABPQNSbuKuZ1yY1+ex3Y=ZY=7cjLwvo3Chbwi7ApoEjUOAErbA@mail.gmail.com>
In-Reply-To: <16520370.401.1319598319120.JavaMail.geo-discussion-forums@prms22>

On Wed, Oct 26, 2011 at 5:05 AM, Atsushi Nakagawa <atnak@chejz.com> wrote:
> On Oct 25, 11:55 pm, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>> [...]
>> +int pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t
>> *attr)
>> +{
>> +       InitializeCriticalSection(&mutex->cs);
>> +       mutex->autoinit = 0;
>> +       return 0;
>> +}
>> +
>> +int pthread_mutex_lock(pthread_mutex_t *mutex)
>> +{
>> +       if (mutex->autoinit) {
>> +               if (InterlockedCompareExchange(&mutex->autoinit, -1, 1) !=
>> -1) {
> I'm making the assumption that mutex->autoinit starts off as 1 before things
> get multi-threaded..
> I've only looked at what's in the patch so I could be missing vital
> context..  Anyways, is there a reason why you made this
> "InterlockedCompareExchange(..., -1, 1) != -1" and not
> "InterlockedCompareExchange(..., -1, 1) == 1"?

No, not really.

> It looks to me the former adds a race condition after "if (mutex->autoinit)
> {".  e.g. A second thread could reinitialize mutex->cs after the first
> thread has already entered EnterCriticalSection(...).

You are indeed correct, thanks for spotting :)