Sergei Golovan <[email protected]> writes:

>> Tcl_FinalizeNotifier() is using following combination
>>
>>         pthread_mutex_lock(&notifierInitMutex);
>>         pthread_cond_wait(&notifierCV, &notifierMutex);
>>         pthread_mutex_unlock(&notifierInitMutex);
>>
>> From history, this is clearly conversion mistake. notifierInitMutex should be
>> notifierMutex (no Init).
>
> I'm not sure that the correct way to fix this is to replace
> notifierInitMutex by notifierMutex. The notifierInitMutex was
> introduced recently, and I think it was intentional. So, maybe to fix
> this we should go the other way: replace notifierMutex by
> notifierInitMutex in pthread_cond_wait? Could you try and comment on
> this as well? I don't have access to hardware with HLE/RTM extensions,
> so can't test myself.

I see. Then another candidate is the following.

         pthread_mutex_lock(&notifierInitMutex);
         pthread_mutex_lock(&notifierMutex);
         pthread_cond_wait(&notifierCV, &notifierMutex);
         pthread_mutex_unlock(&notifierMutex);
         pthread_mutex_unlock(&notifierInitMutex);

Because notifierCV is protected by notifierMutex in other places
(StartNotifierThread and NotifierThreadProc). So without notifierMutex
lock, it would be wrong way (has possibility of race).

So, I tested the following version, and it seems to be working.

---

 debian/changelog    |    6 ++++++
 unix/tclUnixNotfy.c |    2 ++
 2 files changed, 8 insertions(+)

diff -puN unix/tclUnixNotfy.c~fix-mutex-typo unix/tclUnixNotfy.c
--- tcl8.6-8.6.5+dfsg/unix/tclUnixNotfy.c~fix-mutex-typo        2016-03-19 
01:56:33.736475677 +0900
+++ tcl8.6-8.6.5+dfsg-hirofumi/unix/tclUnixNotfy.c      2016-03-20 
20:42:03.421497517 +0900
@@ -433,9 +433,11 @@ Tcl_FinalizeNotifier(
                            "unable to write q to triggerPipe");
                }
                close(triggerPipe);
+               pthread_mutex_lock(&notifierMutex);
                while(triggerPipe != -1) {
                    pthread_cond_wait(&notifierCV, &notifierMutex);
                }
+               pthread_mutex_unlock(&notifierMutex);
                if (notifierThreadRunning) {
                    int result = pthread_join((pthread_t) notifierThread, NULL);
 

-- 
OGAWA Hirofumi <[email protected]>

Reply via email to