Patch for [8bd13f07bde6fb0631f27927e36461fdefe8ca95|8bd13f07bd]: Closing tcl pipes prevents windows threads from starting up. Contributed by "stanko".bug_8bd13f07bd

3 files changed, 169 insertions, 15 deletions

diff --git a/win/tclWinConsole.c b/win/tclWinConsole.c
index ab55035..3c9fef0 100644
--- a/win/tclWinConsole.c
+++ b/win/tclWinConsole.c
@@ -51,6 +51,8 @@ TCL_DECLARE_MUTEX(consoleMutex)
 
 typedef struct ConsoleThreadInfo {
     HANDLE thread;		/* Handle to reader or writer thread. */
+    HANDLE threadInitialized;	/* Manual-reset event to signal that thread has been initialized. */
+    int threadExiting;		/* Boolean indicating that thread is exiting. */
     HANDLE readyEvent;		/* Manual-reset event to signal _to_ the main
 				 * thread when the worker thread has finished
 				 * waiting for its normal work to happen. */
@@ -537,8 +539,11 @@ StartChannelThread(
     threadInfoPtr->readyEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
     threadInfoPtr->startEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
     threadInfoPtr->stopEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
+    threadInfoPtr->threadInitialized = CreateEvent(NULL, TRUE, FALSE, NULL);
+    threadInfoPtr->threadExiting = FALSE;
     threadInfoPtr->thread = CreateThread(NULL, 256, threadProc, infoPtr, 0,
 	    &id);
+    WaitForSingleObject(threadInfoPtr->threadInitialized, INFINITE); /* wait for thread to initialize */
     SetThreadPriority(threadInfoPtr->thread, THREAD_PRIORITY_HIGHEST);
 }
 
@@ -572,11 +577,28 @@ StopChannelThread(
 	     * Note that we need to guard against terminating the thread while
 	     * it is in the middle of Tcl_ThreadAlert because it won't be able
 	     * to release the notifier lock.
+	     *
+	     * Also note that terminating threads during their initialization or teardown phase
+	     * may result in ntdll.dll's LoaderLock to remain locked indefinitely.
+	     * This causes ntdll.dll's LdrpInitializeThread() to deadlock trying to acquire LoaderLock.
+	     * LdrpInitializeThread() is executed within new threads to perform
+	     * initialization and to execute DllMain() of all loaded dlls.
+	     * As a result, all new threads are deadlocked in their initialization phase and never execute,
+	     * even though CreateThread() reports successful thread creation.
+	     * This results in a very weird process-wide behavior, which is extremely hard to debug.
+	     *
+	     * THREADS SHOULD NEVER BE TERMINATED. Period.
+	     *
+	     * But for now, check if thread is exiting, and if so, let it die peacefully.
 	     */
 
 	    Tcl_MutexLock(&consoleMutex);
-	    /* BUG: this leaks memory. */
-	    TerminateThread(threadInfoPtr->thread, 0);
+	    if (threadInfoPtr->threadExiting) {
+		    WaitForSingleObject(threadInfoPtr->thread, INFINITE);
+	    } else {
+		    /* BUG: this leaks memory. */
+		    TerminateThread(threadInfoPtr->thread, 0);
+	    }
 	    Tcl_MutexUnlock(&consoleMutex);
 	}
     }
@@ -587,6 +609,7 @@ StopChannelThread(
      */
 
     CloseHandle(threadInfoPtr->thread);
+    CloseHandle(threadInfoPtr->threadInitialized);
     CloseHandle(threadInfoPtr->readyEvent);
     CloseHandle(threadInfoPtr->startEvent);
     CloseHandle(threadInfoPtr->stopEvent);
@@ -1186,6 +1209,11 @@ ConsoleReaderThread(
     HANDLE wEvents[2];
 
     /*
+     * Notify StartChannelThread() that this thread is initialized
+     */
+    SetEvent(threadInfo->threadInitialized);
+
+    /*
      * The first event takes precedence.
      */
 
@@ -1253,6 +1281,14 @@ ConsoleReaderThread(
 	Tcl_MutexUnlock(&consoleMutex);
     }
 
+    /*
+     * Inform StopChannelThread() that this thread should not be terminated, since it is about to exit.
+     * See comment in StopChannelThread() for reasons.
+    */
+    Tcl_MutexLock(&consoleMutex);
+    threadInfo->threadExiting = TRUE;
+    Tcl_MutexUnlock(&consoleMutex);
+
     return 0;
 }
 
@@ -1287,6 +1323,11 @@ ConsoleWriterThread(
     HANDLE wEvents[2];
 
     /*
+     * Notify StartChannelThread() that this thread is initialized
+     */
+    SetEvent(threadInfo->threadInitialized);
+
+    /*
      * The first event takes precedence.
      */
 
@@ -1351,6 +1392,14 @@ ConsoleWriterThread(
 	Tcl_MutexUnlock(&consoleMutex);
     }
 
+    /*
+     * Inform StopChannelThread() that this thread should not be terminated, since it is about to exit.
+     * See comment in StopChannelThread() for reasons.
+    */
+    Tcl_MutexLock(&consoleMutex);
+    threadInfo->threadExiting = TRUE;
+    Tcl_MutexUnlock(&consoleMutex);
+
     return 0;
 }
 
diff --git a/win/tclWinPipe.c b/win/tclWinPipe.c
index 4666deb..cefd5e6 100644
--- a/win/tclWinPipe.c
+++ b/win/tclWinPipe.c
@@ -111,6 +111,10 @@ typedef struct PipeInfo {
 				 * threads. */
     HANDLE writeThread;		/* Handle to writer thread. */
     HANDLE readThread;		/* Handle to reader thread. */
+    HANDLE writeThreadInitialized;	/* Manual-reset event to signal that writer thread has been initialized */
+    HANDLE readThreadInitialized;	/* Manual-reset event to signal that reader thread has been initialized */
+    int writeThreadExiting;	/* Boolean indicating that write thread is exiting */
+    int readThreadExiting;	/* Boolean indicating that read thread is exiting */
     HANDLE writable;		/* Manual-reset event to signal when the
 				 * writer thread has finished waiting for the
 				 * current buffer to be written. */
@@ -1601,8 +1605,11 @@ TclpCreateCommandChannel(
 	infoPtr->readable = CreateEvent(NULL, TRUE, TRUE, NULL);
 	infoPtr->startReader = CreateEvent(NULL, FALSE, FALSE, NULL);
 	infoPtr->stopReader = CreateEvent(NULL, TRUE, FALSE, NULL);
+	infoPtr->readThreadInitialized = CreateEvent(NULL, TRUE, FALSE, NULL);
+	infoPtr->readThreadExiting = FALSE;
 	infoPtr->readThread = CreateThread(NULL, 256, PipeReaderThread,
 		infoPtr, 0, &id);
+	WaitForSingleObject(infoPtr->readThreadInitialized, INFINITE); /* wait for thread to initialize */
 	SetThreadPriority(infoPtr->readThread, THREAD_PRIORITY_HIGHEST);
 	infoPtr->validMask |= TCL_READABLE;
     } else {
@@ -1616,8 +1623,11 @@ TclpCreateCommandChannel(
 	infoPtr->writable = CreateEvent(NULL, TRUE, TRUE, NULL);
 	infoPtr->startWriter = CreateEvent(NULL, FALSE, FALSE, NULL);
 	infoPtr->stopWriter = CreateEvent(NULL, TRUE, FALSE, NULL);
+	infoPtr->writeThreadInitialized = CreateEvent(NULL, TRUE, FALSE, NULL);
+	infoPtr->writeThreadExiting = FALSE;
 	infoPtr->writeThread = CreateThread(NULL, 256, PipeWriterThread,
 		infoPtr, 0, &id);
+	WaitForSingleObject(infoPtr->writeThreadInitialized, INFINITE); /* wait for thread to initialize */
 	SetThreadPriority(infoPtr->readThread, THREAD_PRIORITY_HIGHEST);
 	infoPtr->validMask |= TCL_WRITABLE;
     }
@@ -1853,17 +1863,34 @@ PipeClose2Proc(
 		     * Note that we need to guard against terminating the
 		     * thread while it is in the middle of Tcl_ThreadAlert
 		     * because it won't be able to release the notifier lock.
+		     *
+		     * Also note that terminating threads during their initialization or teardown phase
+		     * may result in ntdll.dll's LoaderLock to remain locked indefinitely.
+		     * This causes ntdll.dll's LdrpInitializeThread() to deadlock trying to acquire LoaderLock.
+		     * LdrpInitializeThread() is executed within new threads to perform
+		     * initialization and to execute DllMain() of all loaded dlls.
+		     * As a result, all new threads are deadlocked in their initialization phase and never execute,
+		     * even though CreateThread() reports successful thread creation.
+		     * This results in a very weird process-wide behavior, which is extremely hard to debug.
+		     *
+		     * THREADS SHOULD NEVER BE TERMINATED. Period.
+		     *
+		     * But for now, check if thread is exiting, and if so, let it die peacefully.
 		     */
 
 		    Tcl_MutexLock(&pipeMutex);
-
-		    /* BUG: this leaks memory */
-		    TerminateThread(pipePtr->readThread, 0);
+		    if (pipePtr->readThreadExiting) {
+			    WaitForSingleObject(pipePtr->readThread, INFINITE);
+		    } else {
+			    /* BUG: this leaks memory */
+			    TerminateThread(pipePtr->readThread, 0);
+		    }
 		    Tcl_MutexUnlock(&pipeMutex);
 		}
 	    }
 
 	    CloseHandle(pipePtr->readThread);
+	    CloseHandle(pipePtr->readThreadInitialized);
 	    CloseHandle(pipePtr->readable);
 	    CloseHandle(pipePtr->startReader);
 	    CloseHandle(pipePtr->stopReader);
@@ -1909,8 +1936,8 @@ PipeClose2Proc(
 
 	    if (exitCode == STILL_ACTIVE) {
 		/*
-		 * Set the stop event so that if the reader thread is blocked
-		 * in PipeReaderThread on WaitForMultipleEvents, it will exit
+		 * Set the stop event so that if the writer thread is blocked
+		 * in PipeWriterThread on WaitForMultipleEvents, it will exit
 		 * cleanly.
 		 */
 
@@ -1935,17 +1962,34 @@ PipeClose2Proc(
 		     * Note that we need to guard against terminating the
 		     * thread while it is in the middle of Tcl_ThreadAlert
 		     * because it won't be able to release the notifier lock.
+		     *
+		     * Also note that terminating threads during their initialization or teardown phase
+		     * may result in ntdll.dll's LoaderLock to remain locked indefinitely.
+		     * This causes ntdll.dll's LdrpInitializeThread() to deadlock trying to acquire LoaderLock.
+		     * LdrpInitializeThread() is executed within new threads to perform
+		     * initialization and to execute DllMain() of all loaded dlls.
+		     * As a result, all new threads are deadlocked in their initialization phase and never execute,
+		     * even though CreateThread() reports successful thread creation.
+		     * This results in a very weird process-wide behavior, which is extremely hard to debug.
+		     *
+		     * THREADS SHOULD NEVER BE TERMINATED. Period.
+		     *
+		     * But for now, check if thread is exiting, and if so, let it die peacefully.
 		     */
 
 		    Tcl_MutexLock(&pipeMutex);
-
-		    /* BUG: this leaks memory */
-		    TerminateThread(pipePtr->writeThread, 0);
+		    if (pipePtr->writeThreadExiting) {
+			    WaitForSingleObject(pipePtr->writeThread, INFINITE);
+		    } else {
+			    /* BUG: this leaks memory */
+			    TerminateThread(pipePtr->writeThread, 0);
+		    }
 		    Tcl_MutexUnlock(&pipeMutex);
 		}
 	    }
 
 	    CloseHandle(pipePtr->writeThread);
+	    CloseHandle(pipePtr->writeThreadInitialized);
 	    CloseHandle(pipePtr->writable);
 	    CloseHandle(pipePtr->startWriter);
 	    CloseHandle(pipePtr->stopWriter);
@@ -2821,6 +2865,11 @@ PipeReaderThread(
     HANDLE wEvents[2];
     DWORD waitResult;
 
+    /*
+     * Let TclpCreateCommandChannel() know that this thread has been initialized
+     */
+    SetEvent(infoPtr->readThreadInitialized);
+
     wEvents[0] = infoPtr->stopReader;
     wEvents[1] = infoPtr->startReader;
 
@@ -2913,6 +2962,14 @@ PipeReaderThread(
 	Tcl_MutexUnlock(&pipeMutex);
     }
 
+    /*
+     * Inform PipeClose2Proc() that this thread should not be terminated, since it is about to exit.
+     * See comment in PipeClose2Proc() for reasons.
+    */
+    Tcl_MutexLock(&pipeMutex);
+    infoPtr->readThreadExiting = TRUE;
+    Tcl_MutexUnlock(&pipeMutex);
+
     return 0;
 }
 
@@ -2945,6 +3002,11 @@ PipeWriterThread(
     HANDLE wEvents[2];
     DWORD waitResult;
 
+    /*
+     * Let TclpCreateCommandChannel() know that this thread has been initialized
+     */
+    SetEvent(infoPtr->writeThreadInitialized);
+
     wEvents[0] = infoPtr->stopWriter;
     wEvents[1] = infoPtr->startWriter;
 
@@ -3011,6 +3073,14 @@ PipeWriterThread(
 	Tcl_MutexUnlock(&pipeMutex);
     }
 
+    /*
+     * Inform PipeClose2Proc() that this thread should not be terminated, since it is about to exit.
+     * See comment in PipeClose2Proc() for reasons.
+    */
+    Tcl_MutexLock(&pipeMutex);
+    infoPtr->writeThreadExiting = TRUE;
+    Tcl_MutexUnlock(&pipeMutex);
+
     return 0;
 }
 
diff --git a/win/tclWinSerial.c b/win/tclWinSerial.c
index 0730a46..8bb30c5 100644
--- a/win/tclWinSerial.c
+++ b/win/tclWinSerial.c
@@ -94,6 +94,8 @@ typedef struct SerialInfo {
     OVERLAPPED osRead;		/* OVERLAPPED structure for read operations. */
     OVERLAPPED osWrite;		/* OVERLAPPED structure for write operations */
     HANDLE writeThread;		/* Handle to writer thread. */
+    HANDLE writeThreadInitialized;	/* Manual-reset event to signal that thread has been initialized. */
+    int writeThreadExiting;		/* Boolean indicating that thread is exiting. */
     CRITICAL_SECTION csWrite;	/* Writer thread synchronisation. */
     HANDLE evWritable;		/* Manual-reset event to signal when the
 				 * writer thread has finished waiting for the
@@ -643,18 +645,34 @@ SerialCloseProc(
 		 * Note that we need to guard against terminating the thread
 		 * while it is in the middle of Tcl_ThreadAlert because it
 		 * won't be able to release the notifier lock.
+		 *
+		 * Also note that terminating threads during their initialization or teardown phase
+		 * may result in ntdll.dll's LoaderLock to remain locked indefinitely.
+		 * This causes ntdll.dll's LdrpInitializeThread() to deadlock trying to acquire LoaderLock.
+		 * LdrpInitializeThread() is executed within new threads to perform
+		 * initialization and to execute DllMain() of all loaded dlls.
+		 * As a result, all new threads are deadlocked in their initialization phase and never execute,
+		 * even though CreateThread() reports successful thread creation.
+		 * This results in a very weird process-wide behavior, which is extremely hard to debug.
+		 *
+		 * THREADS SHOULD NEVER BE TERMINATED. Period.
+		 *
+		 * But for now, check if thread is exiting, and if so, let it die peacefully.
 		 */
 
 		Tcl_MutexLock(&serialMutex);
-
-		/* BUG: this leaks memory */
-		TerminateThread(serialPtr->writeThread, 0);
-
+		if (serialPtr->writeThreadExiting) {
+			WaitForSingleObject(serialPtr->writeThread, INFINITE);
+		} else {
+			/* BUG: this leaks memory. */
+			TerminateThread(serialPtr->writeThread, 0);
+		}
 		Tcl_MutexUnlock(&serialMutex);
 	    }
 	}
 
 	CloseHandle(serialPtr->writeThread);
+	CloseHandle(serialPtr->writeThreadInitialized);
 	CloseHandle(serialPtr->osWrite.hEvent);
 	CloseHandle(serialPtr->evWritable);
 	CloseHandle(serialPtr->evStartWriter);
@@ -1320,6 +1338,11 @@ SerialWriterThread(
     HANDLE wEvents[2];
 
     /*
+     * Notify TclWinOpenSerialChannel() that this thread is initialized
+     */
+    SetEvent(infoPtr->writeThreadInitialized);
+
+    /*
      * The stop event takes precedence by being first in the list.
      */
 
@@ -1404,6 +1427,15 @@ SerialWriterThread(
 	Tcl_MutexUnlock(&serialMutex);
     }
 
+
+    /*
+     * Inform SerialCloseProc() that this thread should not be terminated, since it is about to exit.
+     * See comment in SerialCloseProc() for reasons.
+    */
+    Tcl_MutexLock(&serialMutex);
+    infoPtr->writeThreadExiting = TRUE;
+    Tcl_MutexUnlock(&serialMutex);
+
     return 0;
 }
 
@@ -1436,7 +1468,7 @@ TclWinSerialOpen(
      * If an open channel is specified, close it
      */
 
-    if ( handle != INVALID_HANDLE_VALUE && CloseHandle(handle) == FALSE) {
+    if (handle != INVALID_HANDLE_VALUE && CloseHandle(handle) == FALSE) {
 	return INVALID_HANDLE_VALUE;
     }
 
@@ -1531,8 +1563,11 @@ TclWinOpenSerialChannel(
 	infoPtr->evWritable = CreateEvent(NULL, TRUE, TRUE, NULL);
 	infoPtr->evStartWriter = CreateEvent(NULL, FALSE, FALSE, NULL);
 	infoPtr->evStopWriter = CreateEvent(NULL, FALSE, FALSE, NULL);
+	infoPtr->writeThreadInitialized = CreateEvent(NULL, TRUE, FALSE, NULL);
+	infoPtr->writeThreadExiting = FALSE;
 	infoPtr->writeThread = CreateThread(NULL, 256, SerialWriterThread,
 		infoPtr, 0, &id);
+	WaitForSingleObject(infoPtr->writeThreadInitialized, INFINITE); /* wait for thread to initialize */
     }
 
     /*