small review: rewritten using already available event handles, additionally prevents infinite waits (using timeout 5000ms);

3 files changed, 94 insertions, 80 deletions

diff --git a/win/tclWinConsole.c b/win/tclWinConsole.c
index 5c0b43b..42bc56f 100644
--- a/win/tclWinConsole.c
+++ b/win/tclWinConsole.c
@@ -51,14 +51,14 @@ 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. */
     HANDLE startEvent;		/* Auto-reset event used by the main thread to
 				 * signal when the thread should attempt to do
-				 * its normal work. */
+				 * its normal work. Additionally this event
+				 * used as wait for thread event (init phase). */
     HANDLE stopEvent;		/* Auto-reset event used by the main thread to
 				 * signal when the thread should exit. */
 } ConsoleThreadInfo;
@@ -538,12 +538,10 @@ StartChannelThread(
 
     threadInfoPtr->readyEvent = CreateEvent(NULL, TRUE, TRUE, NULL);
     threadInfoPtr->startEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
-    threadInfoPtr->threadInitialized = CreateEvent(NULL, TRUE, FALSE, NULL);
     threadInfoPtr->threadExiting = FALSE;
     threadInfoPtr->stopEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
     threadInfoPtr->thread = CreateThread(NULL, 256, threadProc, infoPtr, 0,
 	    &id);
-    WaitForSingleObject(threadInfoPtr->threadInitialized, INFINITE); /* wait for thread to initialize */
     SetThreadPriority(threadInfoPtr->thread, THREAD_PRIORITY_HIGHEST);
 }
 
@@ -592,14 +590,14 @@ StopChannelThread(
 	     * But for now, check if thread is exiting, and if so, let it die peacefully.
 	     */
 
-	    Tcl_MutexLock(&consoleMutex);
-	    if ( threadInfoPtr->threadExiting ) {
-		    WaitForSingleObject(threadInfoPtr->thread, INFINITE);
-	    } else {
-		    /* BUG: this leaks memory. */
-		    TerminateThread(threadInfoPtr->thread, 0);
+	    if ( !threadInfoPtr->threadExiting
+	      || WaitForSingleObject(threadInfoPtr->thread, 5000) != WAIT_OBJECT_0
+	    ) {
+		Tcl_MutexLock(&consoleMutex);
+		/* BUG: this leaks memory. */
+		TerminateThread(threadInfoPtr->thread, 0);
+		Tcl_MutexUnlock(&consoleMutex);
 	    }
-	    Tcl_MutexUnlock(&consoleMutex);
 	}
     }
 
@@ -609,7 +607,6 @@ StopChannelThread(
      */
 
     CloseHandle(threadInfoPtr->thread);
-    CloseHandle(threadInfoPtr->threadInitialized);
     CloseHandle(threadInfoPtr->readyEvent);
     CloseHandle(threadInfoPtr->startEvent);
     CloseHandle(threadInfoPtr->stopEvent);
@@ -1209,9 +1206,10 @@ ConsoleReaderThread(
     HANDLE wEvents[2];
 
     /*
-     * Notify StartChannelThread() that this thread is initialized
+     * Notify caller (using startEvent) that this thread is initialized
      */
-    SetEvent(threadInfo->threadInitialized);
+    SetEvent(threadInfo->startEvent);
+    SuspendThread(threadInfo->thread); /* until main thread get an event */
 
     /*
      * The first event takes precedence.
@@ -1282,12 +1280,10 @@ ConsoleReaderThread(
     }
 
     /*
-     * Inform StopChannelThread() that this thread should not be terminated, since it is about to exit.
+     * Inform caller 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;
 }
@@ -1323,9 +1319,10 @@ ConsoleWriterThread(
     HANDLE wEvents[2];
 
     /*
-     * Notify StartChannelThread() that this thread is initialized
+     * Notify caller (using startEvent) that this thread is initialized
      */
-    SetEvent(threadInfo->threadInitialized);
+    SetEvent(threadInfo->startEvent);
+    SuspendThread(threadInfo->thread); /* until main thread get an event */
 
     /*
      * The first event takes precedence.
@@ -1393,12 +1390,10 @@ ConsoleWriterThread(
     }
 
     /*
-     * Inform StopChannelThread() that this thread should not be terminated, since it is about to exit.
+     * Inform caller 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;
 }
@@ -1429,7 +1424,8 @@ TclWinOpenConsoleChannel(
 {
     char encoding[4 + TCL_INTEGER_SPACE];
     ConsoleInfo *infoPtr;
-    DWORD modes;
+    DWORD modes, wEventsCnt = 0;
+    HANDLE wEvents[2], wEventsPtr = wEvents;
 
     ConsoleInit();
 
@@ -1471,12 +1467,25 @@ TclWinOpenConsoleChannel(
 	modes |= ENABLE_LINE_INPUT;
 	SetConsoleMode(infoPtr->handle, modes);
 	StartChannelThread(infoPtr, &infoPtr->reader, ConsoleReaderThread);
+	wEvents[wEventsCnt++] = infoPtr->reader.startEvent;
     }
 
     if (permissions & TCL_WRITABLE) {
 	StartChannelThread(infoPtr, &infoPtr->writer, ConsoleWriterThread);
+	wEvents[wEventsCnt++] = infoPtr->writer.startEvent;
     }
 
+    /* 
+     * Wait for both threads to initialize (using theirs startEvent)
+     */
+    if (wEventsCnt) {
+	WaitForMultipleObjects(wEventsCnt, wEvents, TRUE, 5000);
+	/* Resume both waiting threads */
+	if (infoPtr->reader.thread)
+	    ResumeThread(infoPtr->reader.thread);
+	if (infoPtr->writer.thread)
+	    ResumeThread(infoPtr->writer.thread);
+    }
     /*
      * Files have default translation of AUTO and ^Z eof char, which means
      * that a ^Z will be accepted as EOF when reading.
diff --git a/win/tclWinPipe.c b/win/tclWinPipe.c
index 9677792..48dcc25 100644
--- a/win/tclWinPipe.c
+++ b/win/tclWinPipe.c
@@ -111,10 +111,8 @@ 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 */
+    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. */
@@ -123,12 +121,14 @@ typedef struct PipeInfo {
 				 * input. */
     HANDLE startWriter;		/* Auto-reset event used by the main thread to
 				 * signal when the writer thread should
-				 * attempt to write to the pipe. */
+				 * attempt to write to the pipe. Additionally
+				 * this event used as wait for thread event (init). */
     HANDLE stopWriter;		/* Manual-reset event used to alert the reader
 				 * thread to fall-out and exit */
     HANDLE startReader;		/* Auto-reset event used by the main thread to
 				 * signal when the reader thread should
-				 * attempt to read from the pipe. */
+				 * attempt to read from the pipe. Additionally
+				 * this event used as wait for thread event (init). */
     HANDLE stopReader;		/* Manual-reset event used to alert the reader
 				 * thread to fall-out and exit */
     DWORD writeError;		/* An error caused by the last background
@@ -1575,7 +1575,8 @@ TclpCreateCommandChannel(
     Tcl_Pid *pidPtr)		/* An array of process identifiers. */
 {
     char channelName[16 + TCL_INTEGER_SPACE];
-    DWORD id;
+    DWORD id, wEventsCnt = 0;
+    HANDLE wEvents[2];
     PipeInfo *infoPtr = ckalloc(sizeof(PipeInfo));
 
     PipeInit();
@@ -1605,13 +1606,12 @@ 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;
+	wEvents[wEventsCnt++] = infoPtr->startReader;
     } else {
 	infoPtr->readThread = 0;
     }
@@ -1623,13 +1623,26 @@ 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;
+	wEvents[wEventsCnt++] = infoPtr->startWriter;
+    } else {
+    	infoPtr->writeThread = 0;
+    }
+
+    /* 
+     * Wait for both threads to initialize (using theirs start-events)
+     */
+    if (wEventsCnt) {
+	WaitForMultipleObjects(wEventsCnt, wEvents, TRUE, 5000);
+	/* Resume both waiting threads */
+	if (infoPtr->readThread)
+	    ResumeThread(infoPtr->readThread);
+	if (infoPtr->writeThread)
+	    ResumeThread(infoPtr->writeThread);
     }
 
     /*
@@ -1878,20 +1891,18 @@ PipeClose2Proc(
 		     * But for now, check if thread is exiting, and if so, let it die peacefully.
 		     */
 
-		    Tcl_MutexLock(&pipeMutex);
-
-		    if ( pipePtr->readThreadExiting ) {
-			    WaitForSingleObject(pipePtr->readThread, INFINITE);
-		    } else {
-			    /* BUG: this leaks memory */
-			    TerminateThread(pipePtr->readThread, 0);
+		    if ( !pipePtr->readThreadExiting
+		      || WaitForSingleObject(pipePtr->readThread, 5000) != WAIT_OBJECT_0
+		    ) {
+			Tcl_MutexLock(&pipeMutex);
+			/* BUG: this leaks memory */
+			TerminateThread(pipePtr->readThread, 0);
+			Tcl_MutexUnlock(&pipeMutex);
 		    }
-		    Tcl_MutexUnlock(&pipeMutex);
 		}
 	    }
 
 	    CloseHandle(pipePtr->readThread);
-	    CloseHandle(pipePtr->readThreadInitialized);
 	    CloseHandle(pipePtr->readable);
 	    CloseHandle(pipePtr->startReader);
 	    CloseHandle(pipePtr->stopReader);
@@ -1978,20 +1989,18 @@ PipeClose2Proc(
 		     * But for now, check if thread is exiting, and if so, let it die peacefully.
 		     */
 
-		    Tcl_MutexLock(&pipeMutex);
-
-		    if ( pipePtr->writeThreadExiting ) {
-			    WaitForSingleObject(pipePtr->writeThread, INFINITE);
-		    } else {
-			    /* BUG: this leaks memory */
-			    TerminateThread(pipePtr->writeThread, 0);
+		    if ( !pipePtr->writeThreadExiting
+		      || WaitForSingleObject(pipePtr->writeThread, 5000) != WAIT_OBJECT_0
+		    ) {
+			Tcl_MutexLock(&pipeMutex);
+			/* BUG: this leaks memory */
+			TerminateThread(pipePtr->writeThread, 0);
+			Tcl_MutexUnlock(&pipeMutex);
 		    }
-		    Tcl_MutexUnlock(&pipeMutex);
 		}
 	    }
 
 	    CloseHandle(pipePtr->writeThread);
-	    CloseHandle(pipePtr->writeThreadInitialized);
 	    CloseHandle(pipePtr->writable);
 	    CloseHandle(pipePtr->startWriter);
 	    CloseHandle(pipePtr->stopWriter);
@@ -2868,9 +2877,10 @@ PipeReaderThread(
     DWORD waitResult;
 
     /*
-     * Let TclpCreateCommandChannel() know that this thread has been initialized
+     * Notify caller that this thread has been initialized
      */
-    SetEvent(infoPtr->readThreadInitialized);
+    SetEvent(infoPtr->startReader);
+    SuspendThread(infoPtr->readThread); /* until main thread get an event */
     
     wEvents[0] = infoPtr->stopReader;
     wEvents[1] = infoPtr->startReader;
@@ -2965,12 +2975,10 @@ PipeReaderThread(
     }
 
     /*
-     * Inform PipeClose2Proc() that this thread should not be terminated, since it is about to exit.
+     * Inform caller 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;
 }
@@ -3005,9 +3013,10 @@ PipeWriterThread(
     DWORD waitResult;
 
     /*
-     * Let TclpCreateCommandChannel() know that this thread has been initialized
+     * Notify caller that this thread has been initialized
      */
-    SetEvent(infoPtr->writeThreadInitialized);
+    SetEvent(infoPtr->startWriter);
+    SuspendThread(infoPtr->writeThread); /* until main thread get an event */
 
     wEvents[0] = infoPtr->stopWriter;
     wEvents[1] = infoPtr->startWriter;
@@ -3076,12 +3085,10 @@ PipeWriterThread(
     }
 
     /*
-     * Inform PipeClose2Proc() that this thread should not be terminated, since it is about to exit.
+     * Inform caller 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 3c6a3cc..fa135ab 100644
--- a/win/tclWinSerial.c
+++ b/win/tclWinSerial.c
@@ -94,15 +94,15 @@ 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. */
+    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
 				 * current buffer to be written. */
     HANDLE evStartWriter;	/* Auto-reset event used by the main thread to
 				 * signal when the writer thread should
-				 * attempt to write to the serial. */
+				 * attempt to write to the serial. Additionally
+				 * this event used as wait for thread event (init). */
     HANDLE evStopWriter;	/* Auto-reset event used by the main thread to
 				 * signal when the writer thread should close.
 				 */
@@ -660,20 +660,18 @@ SerialCloseProc(
 		 * But for now, check if thread is exiting, and if so, let it die peacefully.
 		 */
 
-		Tcl_MutexLock(&serialMutex);
-
-		if ( serialPtr->writeThreadExiting ) {
-			WaitForSingleObject(serialPtr->writeThread, INFINITE);
-		} else {
-			/* BUG: this leaks memory. */
-			TerminateThread(serialPtr->writeThread, 0);
+		if ( !serialPtr->writeThreadExiting
+		  || WaitForSingleObject(serialPtr->writeThread, 5000) != WAIT_OBJECT_0
+		) {
+		    Tcl_MutexLock(&serialMutex);
+		    /* BUG: this leaks memory. */
+		    TerminateThread(serialPtr->writeThread, 0);
+		    Tcl_MutexUnlock(&serialMutex);
 		}
-		Tcl_MutexUnlock(&serialMutex);
 	    }
 	}
 
 	CloseHandle(serialPtr->writeThread);
-	CloseHandle(serialPtr->writeThreadInitialized);
 	CloseHandle(serialPtr->osWrite.hEvent);
 	CloseHandle(serialPtr->evWritable);
 	CloseHandle(serialPtr->evStartWriter);
@@ -1341,7 +1339,8 @@ SerialWriterThread(
     /*
      * Notify TclWinOpenSerialChannel() that this thread is initialized
      */
-    SetEvent(infoPtr->writeThreadInitialized);
+    SetEvent(infoPtr->evStartWriter);
+    SuspendThread(infoPtr->writeThread); /* until main thread get an event */
 
     /*
      * The stop event takes precedence by being first in the list.
@@ -1429,12 +1428,10 @@ SerialWriterThread(
     }
 
     /*
-     * Inform SerialCloseProc() that this thread should not be terminated, since it is about to exit.
+     * Inform caller 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;
 }
@@ -1563,11 +1560,12 @@ 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 */
+	/* Wait for thread to initialize (using evStartWriter) */
+	WaitForSingleObject(infoPtr->evStartWriter, 5000);
+	ResumeThread(infoPtr->writeThread);
     }
 
     /*