[Pkg-tcltk-devel] Bug#818697: Bug#818697: libtcl8.6: Segv in Tcl_FinalizeNotifier()

[OGAWA Hirofumi]

Sergei Golovan <sgolovan at nes.ru> 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 <hirofumi at mail.parknet.co.jp>