Tcl Source Code

View Ticket
Login
Ticket UUID: d3071887dbc7aeac5f5f71741239720e9f72584c
Title: Fix SEGV in Tcl_FinalizeNotifier()
Type: Bug Version: 8.6.5
Submitter: anonymous Created on: 2016-03-18 16:46:22
Subsystem: 49. Threading Assigned To: jan.nijtmans
Priority: 5 Medium Severity: Important
Status: Closed Last Modified: 2016-03-21 12:12:15
Resolution: Fixed Closed By: jan.nijtmans
    Closed on: 2016-03-21 12:12:15
Description:
When closing wish, process die at 

		    pthread_cond_wait(&notifierCV, &notifierMutex);

if cpu is supporting HLE/RTM. (xend instruction doesn't allow to use without
xbegin)

I.e. Tcl_FinalizeNotifier() is using 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).

---

 unix/tclUnixNotfy.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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:25:29.900145502 +0900
+++ tcl8.6-8.6.5+dfsg-hirofumi/unix/tclUnixNotfy.c	2016-03-19 01:24:20.795871648 +0900
@@ -417,7 +417,7 @@ Tcl_FinalizeNotifier(
 #ifdef TCL_THREADS
 	ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey);
 
-	pthread_mutex_lock(&notifierInitMutex);
+	pthread_mutex_lock(&notifierMutex);
 	notifierCount--;
 
 	/*
@@ -460,7 +460,7 @@ Tcl_FinalizeNotifier(
 #endif /* __CYGWIN__ */
 	tsdPtr->waitCVinitialized = 0;
 
-	pthread_mutex_unlock(&notifierInitMutex);
+	pthread_mutex_unlock(&notifierMutex);
 #endif /* TCL_THREADS */
     }
 }
_
User Comments: jan.nijtmans added on 2016-03-21 12:12:15:
Fixed now in all active branches. Thanks for the report and the fix!

anonymous (claiming to be [email protected]) added on 2016-03-20 12:13:48:
Then another (better) candidate is the following, instead to remove
notiferInitMutex protection.

         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). And notiferInitMutex
protects "notifierCount", etc.

I tested the following, 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);