From 992af585dd13c64af68e50f9cad503afb6f0b1ed Mon Sep 17 00:00:00 2001 From: sebres Date: Tue, 11 Apr 2017 18:09:52 +0000 Subject: code review, robustness increase, avoid infinite wait by exit, thread exit and by pipes of closed processes); use pipe-helpers (TI-structure handling) for all pipe-workers (tclWinConsole, tclWinSerial); --- win/tclWinConsole.c | 281 ++++++++++++++-------------------------------------- win/tclWinInt.h | 8 ++ win/tclWinPipe.c | 51 ++++++---- win/tclWinSerial.c | 124 +++-------------------- 4 files changed, 132 insertions(+), 332 deletions(-) diff --git a/win/tclWinConsole.c b/win/tclWinConsole.c index da995c5..7a0965d 100644 --- a/win/tclWinConsole.c +++ b/win/tclWinConsole.c @@ -51,16 +51,10 @@ TCL_DECLARE_MUTEX(consoleMutex) typedef struct ConsoleThreadInfo { HANDLE thread; /* Handle to reader or writer thread. */ - 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. 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. */ + TclPipeThreadInfo *TI; /* Thread info structure of writer and reader. */ } ConsoleThreadInfo; /* @@ -84,16 +78,14 @@ typedef struct ConsoleInfo { * threads. */ ConsoleThreadInfo writer; /* A specialized thread for handling * asynchronous writes to the console; the - * waiting starts when a start event is sent, + * waiting starts when a control event is sent, * and a reset event is sent back to the main - * thread when the write is done. A stop event - * is used to terminate the thread. */ + * thread when the write is done. */ ConsoleThreadInfo reader; /* A specialized thread for handling * asynchronous reads from the console; the - * waiting starts when a start event is sent, + * waiting starts when a control event is sent, * and a reset event is sent back to the main - * thread when input is available. A stop - * event is used to terminate the thread. */ + * thread when input is available. */ DWORD writeError; /* An error caused by the last background * write. Set to 0 if no error has been * detected. This word is shared with the @@ -170,10 +162,6 @@ static BOOL ReadConsoleBytes(HANDLE hConsole, LPVOID lpBuffer, static BOOL WriteConsoleBytes(HANDLE hConsole, const void *lpBuffer, DWORD nbytes, LPDWORD nbyteswritten); -static void StartChannelThread(ConsoleInfo *infoPtr, - ConsoleThreadInfo *threadInfoPtr, - LPTHREAD_START_ROUTINE threadProc); -static void StopChannelThread(ConsoleThreadInfo *threadInfoPtr); /* * This structure describes the channel type structure for command console @@ -520,102 +508,6 @@ ConsoleBlockModeProc( /* *---------------------------------------------------------------------- * - * StartChannelThread, StopChannelThread -- - * - * Helpers that codify how to ask one of the console service threads to - * start and stop. - * - *---------------------------------------------------------------------- - */ - -static void -StartChannelThread( - ConsoleInfo *infoPtr, - ConsoleThreadInfo *threadInfoPtr, - LPTHREAD_START_ROUTINE threadProc) -{ - DWORD id; - - threadInfoPtr->readyEvent = CreateEvent(NULL, TRUE, TRUE, NULL); - threadInfoPtr->startEvent = CreateEvent(NULL, FALSE, FALSE, NULL); - threadInfoPtr->threadExiting = FALSE; - threadInfoPtr->stopEvent = CreateEvent(NULL, FALSE, FALSE, NULL); - threadInfoPtr->thread = CreateThread(NULL, 256, threadProc, infoPtr, 0, - &id); - SetThreadPriority(threadInfoPtr->thread, THREAD_PRIORITY_HIGHEST); -} - -static void -StopChannelThread( - ConsoleThreadInfo *threadInfoPtr) -{ - DWORD exitCode = 0; - - /* - * The thread may already have closed on it's own. Check it's exit - * code. - */ - - GetExitCodeThread(threadInfoPtr->thread, &exitCode); - if (exitCode == STILL_ACTIVE) { - /* - * Set the stop event so that if the reader thread is blocked in - * ConsoleReaderThread on WaitForMultipleEvents, it will exit cleanly. - */ - - SetEvent(threadInfoPtr->stopEvent); - - /* - * Wait at most 20 milliseconds for the reader thread to close. - */ - - if (WaitForSingleObject(threadInfoPtr->thread, 20) == WAIT_TIMEOUT) { - /* - * Forcibly terminate the background thread as a last resort. - * 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. - */ - - if ( !threadInfoPtr->threadExiting - || WaitForSingleObject(threadInfoPtr->thread, 5000) != WAIT_OBJECT_0 - ) { - Tcl_MutexLock(&consoleMutex); - /* BUG: this leaks memory. */ - TerminateThread(threadInfoPtr->thread, 0); - Tcl_MutexUnlock(&consoleMutex); - } - } - } - - /* - * Close all the handles associated with the thread, and set the thread - * handle field to NULL to mark that the thread has been cleaned up. - */ - - CloseHandle(threadInfoPtr->thread); - CloseHandle(threadInfoPtr->readyEvent); - CloseHandle(threadInfoPtr->startEvent); - CloseHandle(threadInfoPtr->stopEvent); - threadInfoPtr->thread = NULL; -} - -/* - *---------------------------------------------------------------------- - * * ConsoleCloseProc -- * * Closes a console based IO channel. @@ -646,7 +538,10 @@ ConsoleCloseProc( */ if (consolePtr->reader.thread) { - StopChannelThread(&consolePtr->reader); + TclPipeThreadStop(&consolePtr->reader.TI, consolePtr->reader.thread); + CloseHandle(consolePtr->reader.thread); + CloseHandle(consolePtr->reader.readyEvent); + consolePtr->reader.thread = NULL; } consolePtr->validMask &= ~TCL_READABLE; @@ -663,10 +558,13 @@ ConsoleCloseProc( * prevent infinite wait on exit. [Python Bug 216289] */ - WaitForSingleObject(consolePtr->writer.readyEvent, INFINITE); + WaitForSingleObject(consolePtr->writer.readyEvent, 5000); } - StopChannelThread(&consolePtr->writer); + TclPipeThreadStop(&consolePtr->writer.TI, consolePtr->writer.thread); + CloseHandle(consolePtr->writer.thread); + CloseHandle(consolePtr->writer.readyEvent); + consolePtr->writer.thread = NULL; } consolePtr->validMask &= ~TCL_WRITABLE; @@ -832,8 +730,11 @@ ConsoleOutputProc( DWORD bytesWritten, timeout; *errorCode = 0; - timeout = (infoPtr->flags & CONSOLE_ASYNC) ? 0 : INFINITE; - if (WaitForSingleObject(threadInfo->readyEvent,timeout) == WAIT_TIMEOUT) { + + /* avoid blocking if pipe-thread exited */ + timeout = (infoPtr->flags & CONSOLE_ASYNC) || !TclPipeThreadIsAlive(&threadInfo->TI) + || TclInExit() || TclInThreadExit() ? 0 : INFINITE; + if (WaitForSingleObject(threadInfo->readyEvent, timeout) == WAIT_TIMEOUT) { /* * The writer thread is blocked waiting for a write to complete and * the channel is in non-blocking mode. @@ -873,7 +774,7 @@ ConsoleOutputProc( memcpy(infoPtr->writeBuf, buf, (size_t) toWrite); infoPtr->toWrite = toWrite; ResetEvent(threadInfo->readyEvent); - SetEvent(threadInfo->startEvent); + TclPipeThreadSignal(&threadInfo->TI); bytesWritten = toWrite; } else { /* @@ -1110,9 +1011,10 @@ WaitForRead( * Synchronize with the reader thread. */ - timeout = blocking ? INFINITE : 0; - if (WaitForSingleObject(threadInfo->readyEvent, - timeout) == WAIT_TIMEOUT) { + /* avoid blocking if pipe-thread exited */ + timeout = (!blocking || !TclPipeThreadIsAlive(&threadInfo->TI) + || TclInExit() || TclInThreadExit()) ? 0 : INFINITE; + if (WaitForSingleObject(threadInfo->readyEvent, timeout) == WAIT_TIMEOUT) { /* * The reader thread is blocked waiting for data and the channel * is in non-blocking mode. @@ -1172,7 +1074,7 @@ WaitForRead( */ ResetEvent(threadInfo->readyEvent); - SetEvent(threadInfo->startEvent); + TclPipeThreadSignal(&threadInfo->TI); } } @@ -1199,39 +1101,27 @@ static DWORD WINAPI ConsoleReaderThread( LPVOID arg) { - ConsoleInfo *infoPtr = arg; - HANDLE *handle = infoPtr->handle; - ConsoleThreadInfo *threadInfo = &infoPtr->reader; - DWORD waitResult; - HANDLE wEvents[2]; - - /* - * Notify caller (using startEvent) that this thread is initialized - */ - SignalObjectAndWait(threadInfo->startEvent, threadInfo->stopEvent, INFINITE, FALSE); - - /* - * The first event takes precedence. - */ - - wEvents[0] = threadInfo->stopEvent; - wEvents[1] = threadInfo->startEvent; - - for (;;) { + TclPipeThreadInfo *pipeTI = (TclPipeThreadInfo *)arg; + ConsoleInfo *infoPtr = NULL; /* access info only after success init/wait */ + HANDLE *handle = NULL; + ConsoleThreadInfo *threadInfo = NULL; + int done = 0; + + while (!done) { /* - * Wait for the main thread to signal before attempting to wait. + * Wait for the main thread to signal before attempting to read. */ - waitResult = WaitForMultipleObjects(2, wEvents, FALSE, INFINITE); - - if (waitResult != (WAIT_OBJECT_0 + 1)) { - /* - * The start event was not signaled. It must be the stop event or - * an error, so exit this thread. - */ - + if (!TclPipeThreadWaitForSignal(&pipeTI)) { + /* exit */ break; } + if (!infoPtr) { + infoPtr = (ConsoleInfo *)pipeTI->clientData; + handle = infoPtr->handle; + threadInfo = &infoPtr->reader; + } + /* * Look for data on the console, but first ignore any events that are @@ -1251,6 +1141,7 @@ ConsoleReaderThread( if (err == (DWORD) EOF) { infoPtr->readFlags = CONSOLE_EOF; } + done = 1; } /* @@ -1278,11 +1169,8 @@ ConsoleReaderThread( Tcl_MutexUnlock(&consoleMutex); } - /* - * Inform caller that this thread should not be terminated, since it is about to exit. - * See comment in StopChannelThread() for reasons. - */ - threadInfo->threadExiting = TRUE; + /* Worker exit, so inform the main thread or free TI-structure (if owned) */ + TclPipeThreadExit(&pipeTI); return 0; } @@ -1310,40 +1198,27 @@ static DWORD WINAPI ConsoleWriterThread( LPVOID arg) { - ConsoleInfo *infoPtr = arg; - HANDLE *handle = infoPtr->handle; - ConsoleThreadInfo *threadInfo = &infoPtr->writer; - DWORD count, toWrite, waitResult; + TclPipeThreadInfo *pipeTI = (TclPipeThreadInfo *)arg; + ConsoleInfo *infoPtr = NULL; /* access info only after success init/wait */ + HANDLE *handle = NULL; + ConsoleThreadInfo *threadInfo = NULL; + DWORD count, toWrite; char *buf; - HANDLE wEvents[2]; - - /* - * Notify caller (using startEvent) that this thread is initialized - */ - SignalObjectAndWait(threadInfo->startEvent, threadInfo->stopEvent, INFINITE, FALSE); - - /* - * The first event takes precedence. - */ - - wEvents[0] = threadInfo->stopEvent; - wEvents[1] = threadInfo->startEvent; - - for (;;) { + int done = 0; + + while (!done) { /* * Wait for the main thread to signal before attempting to write. */ - - waitResult = WaitForMultipleObjects(2, wEvents, FALSE, INFINITE); - - if (waitResult != (WAIT_OBJECT_0 + 1)) { - /* - * The start event was not signaled. It must be the stop event or - * an error, so exit this thread. - */ - + if (!TclPipeThreadWaitForSignal(&pipeTI)) { + /* exit */ break; } + if (!infoPtr) { + infoPtr = (ConsoleInfo *)pipeTI->clientData; + handle = infoPtr->handle; + threadInfo = &infoPtr->writer; + } buf = infoPtr->writeBuf; toWrite = infoPtr->toWrite; @@ -1356,6 +1231,7 @@ ConsoleWriterThread( if (WriteConsoleBytes(handle, buf, (DWORD) toWrite, &count) == FALSE) { infoPtr->writeError = GetLastError(); + done = 1; break; } toWrite -= count; @@ -1387,11 +1263,8 @@ ConsoleWriterThread( Tcl_MutexUnlock(&consoleMutex); } - /* - * Inform caller that this thread should not be terminated, since it is about to exit. - * See comment in StopChannelThread() for reasons. - */ - threadInfo->threadExiting = TRUE; + /* Worker exit, so inform the main thread or free TI-structure (if owned) */ + TclPipeThreadExit(&pipeTI); return 0; } @@ -1422,8 +1295,7 @@ TclWinOpenConsoleChannel( { char encoding[4 + TCL_INTEGER_SPACE]; ConsoleInfo *infoPtr; - DWORD modes, wEventsCnt = 0; - HANDLE wEvents[2], wEventsPtr = wEvents; + DWORD modes; ConsoleInit(); @@ -1464,26 +1336,23 @@ TclWinOpenConsoleChannel( modes &= ~(ENABLE_WINDOW_INPUT | ENABLE_MOUSE_INPUT); modes |= ENABLE_LINE_INPUT; SetConsoleMode(infoPtr->handle, modes); - StartChannelThread(infoPtr, &infoPtr->reader, ConsoleReaderThread); - wEvents[wEventsCnt++] = infoPtr->reader.startEvent; + + infoPtr->reader.readyEvent = CreateEvent(NULL, TRUE, TRUE, NULL); + infoPtr->reader.thread = CreateThread(NULL, 256, ConsoleReaderThread, + TclPipeThreadCreateTI(&infoPtr->reader.TI, infoPtr, + infoPtr->reader.readyEvent), 0, NULL); + SetThreadPriority(infoPtr->reader.thread, THREAD_PRIORITY_HIGHEST); } 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, we've get the events */ - if (infoPtr->reader.thread) - SetEvent(infoPtr->reader.stopEvent); - if (infoPtr->writer.thread) - SetEvent(infoPtr->writer.stopEvent); + infoPtr->writer.readyEvent = CreateEvent(NULL, TRUE, TRUE, NULL); + infoPtr->writer.thread = CreateThread(NULL, 256, ConsoleWriterThread, + TclPipeThreadCreateTI(&infoPtr->writer.TI, infoPtr, + infoPtr->writer.readyEvent), 0, NULL); + SetThreadPriority(infoPtr->writer.thread, THREAD_PRIORITY_HIGHEST); } + /* * 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/tclWinInt.h b/win/tclWinInt.h index 67b00a8..86945a9 100644 --- a/win/tclWinInt.h +++ b/win/tclWinInt.h @@ -149,6 +149,14 @@ TclPipeThreadSignal( } }; +static inline int +TclPipeThreadIsAlive( + TclPipeThreadInfo **pipeTIPtr) +{ + TclPipeThreadInfo *pipeTI = *pipeTIPtr; + return (pipeTI && pipeTI->state != PTI_STATE_DOWN); +}; + MODULE_SCOPE int TclPipeThreadStopSignal(TclPipeThreadInfo **pipeTIPtr, HANDLE wakeEvent); MODULE_SCOPE void TclPipeThreadStop(TclPipeThreadInfo **pipeTIPtr, HANDLE hThread); MODULE_SCOPE void TclPipeThreadExit(TclPipeThreadInfo **pipeTIPtr); diff --git a/win/tclWinPipe.c b/win/tclWinPipe.c index 528f950..87ce790 100644 --- a/win/tclWinPipe.c +++ b/win/tclWinPipe.c @@ -1592,7 +1592,8 @@ TclpCreateCommandChannel( infoPtr->readable = CreateEvent(NULL, TRUE, TRUE, NULL); infoPtr->readThread = CreateThread(NULL, 256, PipeReaderThread, - TclPipeThreadCreateTI(&infoPtr->readTI, infoPtr, NULL), 0, NULL); + TclPipeThreadCreateTI(&infoPtr->readTI, infoPtr, infoPtr->readable), + 0, NULL); SetThreadPriority(infoPtr->readThread, THREAD_PRIORITY_HIGHEST); infoPtr->validMask |= TCL_READABLE; } else { @@ -1798,6 +1799,7 @@ PipeClose2Proc( int errorCode, result; PipeInfo *infoPtr, **nextPtrPtr; ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); + int inExit = (TclInExit() || TclInThreadExit()); errorCode = 0; result = 0; @@ -1832,7 +1834,7 @@ PipeClose2Proc( * nonblocking or may block during exit, bail out since the worker * thread is not interruptible and we want TIP#398-fast-exit. */ - if ((pipePtr->flags & PIPE_ASYNC) && TclInExit()) { + if ((pipePtr->flags & PIPE_ASYNC) && inExit) { /* give it a chance to leave honorably */ TclPipeThreadStopSignal(&pipePtr->writeTI, pipePtr->writable); @@ -1843,7 +1845,7 @@ PipeClose2Proc( } else { - WaitForSingleObject(pipePtr->writable, INFINITE); + WaitForSingleObject(pipePtr->writable, inExit ? 5000 : INFINITE); } @@ -1885,7 +1887,7 @@ PipeClose2Proc( } } - if ((pipePtr->flags & PIPE_ASYNC) || TclInExit()) { + if ((pipePtr->flags & PIPE_ASYNC) || inExit) { /* * If the channel is non-blocking or Tcl is being cleaned up, just * detach the children PIDs, reap them (important if we are in a @@ -2063,7 +2065,10 @@ PipeOutputProc( DWORD bytesWritten, timeout; *errorCode = 0; - timeout = (infoPtr->flags & PIPE_ASYNC) ? 0 : INFINITE; + + /* avoid blocking if pipe-thread exited */ + timeout = ((infoPtr->flags & PIPE_ASYNC) || !TclPipeThreadIsAlive(&infoPtr->writeTI) + || TclInExit() || TclInThreadExit()) ? 0 : INFINITE; if (WaitForSingleObject(infoPtr->writable, timeout) == WAIT_TIMEOUT) { /* * The writer thread is blocked waiting for a write to complete and @@ -2614,7 +2619,9 @@ WaitForRead( * Synchronize with the reader thread. */ - timeout = blocking ? INFINITE : 0; + /* avoid blocking if pipe-thread exited */ + timeout = (!blocking || !TclPipeThreadIsAlive(&infoPtr->readTI) + || TclInExit() || TclInThreadExit()) ? 0 : INFINITE; if (WaitForSingleObject(infoPtr->readable, timeout) == WAIT_TIMEOUT) { /* * The reader thread is blocked waiting for data and the channel @@ -2756,7 +2763,7 @@ PipeReaderThread( infoPtr->readFlags |= PIPE_EOF; done = 1; } else if (err == ERROR_INVALID_HANDLE) { - break; + done = 1; } } else if (count == 0) { if (ReadFile(handle, &(infoPtr->extraByte), 1, &count, NULL) @@ -2778,7 +2785,7 @@ PipeReaderThread( infoPtr->readFlags |= PIPE_EOF; done = 1; } else if (err == ERROR_INVALID_HANDLE) { - break; + done = 1; } } } @@ -3247,7 +3254,7 @@ TclPipeThreadStop( HANDLE hThread) { TclPipeThreadInfo *pipeTI = *pipeTIPtr; - HANDLE evControl; + HANDLE evControl, wakeEvent; int state; if (!pipeTI) { @@ -3255,6 +3262,7 @@ TclPipeThreadStop( } pipeTI = *pipeTIPtr; evControl = pipeTI->evControl; + wakeEvent = pipeTI->evWakeUp; pipeTI->evWakeUp = NULL; /* * Try to sane stop the pipe worker, corresponding its current state @@ -3290,7 +3298,12 @@ TclPipeThreadStop( * Thread works currently, we should try to end it, own the TI structure * (because of possible sharing the joint structures with thread) */ - InterlockedExchange(&pipeTI->state, PTI_STATE_END); + if ((state = InterlockedCompareExchange(&pipeTI->state, + PTI_STATE_END, PTI_STATE_WORK)) == PTI_STATE_DOWN + ) { + /* we don't need to wait for it, but we should free pipeTI */ + hThread = NULL; + }; break; } @@ -3305,6 +3318,8 @@ TclPipeThreadStop( GetExitCodeThread(hThread, &exitCode); if (exitCode == STILL_ACTIVE) { + + int inExit = (TclInExit() || TclInThreadExit()); /* * Set the stop event so that if the pipe thread is blocked * somewhere, it may hereafter sane exit cleanly. @@ -3325,8 +3340,7 @@ TclPipeThreadStop( */ /* if we want TIP#398-fast-exit. */ - if (WaitForSingleObject(hThread, - TclInExit() ? 0 : 20) == WAIT_TIMEOUT) { + if (WaitForSingleObject(hThread, inExit ? 0 : 20) == WAIT_TIMEOUT) { /* * The thread must be blocked waiting for the pipe to @@ -3353,22 +3367,22 @@ TclPipeThreadStop( * THREADS SHOULD NEVER BE TERMINATED. Period. * * But for now, check if thread is exiting, and if so, let it die peacefully. + * + * Also don't terminate if in exit (otherwise deadlocked in ntdll.dll's). */ if ( pipeTI->state != PTI_STATE_DOWN && WaitForSingleObject(hThread, - TclInExit() ? 0 : 5000) != WAIT_OBJECT_0 + inExit ? 50 : 5000) != WAIT_OBJECT_0 ) { - Tcl_MutexLock(&pipeMutex); /* BUG: this leaks memory */ - if (!TerminateThread(hThread, 0)) { - /* terminate fails, just give thread a chance to exit */ + if (inExit || !TerminateThread(hThread, 0)) { + /* in exit or terminate fails, just give thread a chance to exit */ if (InterlockedExchange(&pipeTI->state, PTI_STATE_STOP) != PTI_STATE_DOWN) { pipeTI = NULL; } }; - Tcl_MutexUnlock(&pipeMutex); } } } @@ -3376,6 +3390,9 @@ TclPipeThreadStop( *pipeTIPtr = NULL; if (pipeTI) { + if (pipeTI->evWakeUp) { + SetEvent(pipeTI->evWakeUp); + } CloseHandle(pipeTI->evControl); #ifndef _PTI_USE_CKALLOC free(pipeTI); diff --git a/win/tclWinSerial.c b/win/tclWinSerial.c index f55f5f1..f78aa5a 100644 --- a/win/tclWinSerial.c +++ b/win/tclWinSerial.c @@ -93,19 +93,12 @@ typedef struct SerialInfo { * threads. */ OVERLAPPED osRead; /* OVERLAPPED structure for read operations. */ OVERLAPPED osWrite; /* OVERLAPPED structure for write operations */ + TclPipeThreadInfo *writeTI; /* Thread info structure of writer worker. */ HANDLE writeThread; /* Handle to writer thread. */ - 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. 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. - */ DWORD writeError; /* An error caused by the last background * write. Set to 0 if no error has been * detected. This word is shared with the @@ -601,7 +594,6 @@ SerialCloseProc( int errorCode, result = 0; SerialInfo *infoPtr, **nextPtrPtr; ThreadSpecificData *tsdPtr = TCL_TSD_INIT(&dataKey); - DWORD exitCode; errorCode = 0; @@ -611,71 +603,13 @@ SerialCloseProc( } serialPtr->validMask &= ~TCL_READABLE; - if (serialPtr->validMask & TCL_WRITABLE) { - /* - * Generally we cannot wait for a pending write operation because it - * may hang due to handshake - * WaitForSingleObject(serialPtr->evWritable, INFINITE); - */ - - /* - * The thread may have already closed on it's own. Check it's exit - * code. - */ - - GetExitCodeThread(serialPtr->writeThread, &exitCode); - - if (exitCode == STILL_ACTIVE) { - /* - * Set the stop event so that if the writer thread is blocked in - * SerialWriterThread on WaitForMultipleEvents, it will exit - * cleanly. - */ + if (serialPtr->writeThread) { - SetEvent(serialPtr->evStopWriter); + TclPipeThreadStop(&serialPtr->writeTI, serialPtr->writeThread); - /* - * Wait at most 20 milliseconds for the writer thread to close. - */ - - if (WaitForSingleObject(serialPtr->writeThread, - 20) == WAIT_TIMEOUT) { - /* - * Forcibly terminate the background thread as a last resort. - * 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. - */ - - if ( !serialPtr->writeThreadExiting - || WaitForSingleObject(serialPtr->writeThread, 5000) != WAIT_OBJECT_0 - ) { - Tcl_MutexLock(&serialMutex); - /* BUG: this leaks memory. */ - TerminateThread(serialPtr->writeThread, 0); - Tcl_MutexUnlock(&serialMutex); - } - } - } - - CloseHandle(serialPtr->writeThread); CloseHandle(serialPtr->osWrite.hEvent); CloseHandle(serialPtr->evWritable); - CloseHandle(serialPtr->evStartWriter); - CloseHandle(serialPtr->evStopWriter); + CloseHandle(serialPtr->writeThread); serialPtr->writeThread = NULL; PurgeComm(serialPtr->handle, PURGE_TXABORT | PURGE_TXCLEAR); @@ -1093,7 +1027,7 @@ SerialOutputProc( memcpy(infoPtr->writeBuf, buf, (size_t) toWrite); infoPtr->toWrite = toWrite; ResetEvent(infoPtr->evWritable); - SetEvent(infoPtr->evStartWriter); + TclPipeThreadSignal(&infoPtr->writeTI); bytesWritten = (DWORD) toWrite; } else { @@ -1330,39 +1264,21 @@ static DWORD WINAPI SerialWriterThread( LPVOID arg) { - SerialInfo *infoPtr = (SerialInfo *)arg; - DWORD bytesWritten, toWrite, waitResult; + TclPipeThreadInfo *pipeTI = (TclPipeThreadInfo *)arg; + SerialInfo *infoPtr = NULL; /* access info only after success init/wait */ + DWORD bytesWritten, toWrite; char *buf; OVERLAPPED myWrite; /* Have an own OVERLAPPED in this thread. */ - HANDLE wEvents[2]; - - /* - * Notify TclWinOpenSerialChannel() that this thread is initialized - */ - SignalObjectAndWait(infoPtr->evStartWriter, infoPtr->evStopWriter, INFINITE, FALSE); - - /* - * The stop event takes precedence by being first in the list. - */ - - wEvents[0] = infoPtr->evStopWriter; - wEvents[1] = infoPtr->evStartWriter; for (;;) { /* * Wait for the main thread to signal before attempting to write. */ - - waitResult = WaitForMultipleObjects(2, wEvents, FALSE, INFINITE); - - if (waitResult != (WAIT_OBJECT_0 + 1)) { - /* - * The start event was not signaled. It might be the stop event or - * an error, so exit. - */ - + if (!TclPipeThreadWaitForSignal(&pipeTI)) { + /* exit */ break; } + infoPtr = (SerialInfo *)pipeTI->clientData; buf = infoPtr->writeBuf; toWrite = infoPtr->toWrite; @@ -1426,11 +1342,8 @@ SerialWriterThread( Tcl_MutexUnlock(&serialMutex); } - /* - * Inform caller that this thread should not be terminated, since it is about to exit. - * See comment in SerialCloseProc() for reasons. - */ - infoPtr->writeThreadExiting = TRUE; + /* Worker exit, so inform the main thread or free TI-structure (if owned) */ + TclPipeThreadExit(&pipeTI); return 0; } @@ -1505,7 +1418,6 @@ TclWinOpenSerialChannel( int permissions) { SerialInfo *infoPtr; - DWORD id; SerialInit(); @@ -1557,15 +1469,9 @@ TclWinOpenSerialChannel( infoPtr->osWrite.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); infoPtr->evWritable = CreateEvent(NULL, TRUE, TRUE, NULL); - infoPtr->evStartWriter = CreateEvent(NULL, FALSE, FALSE, NULL); - infoPtr->evStopWriter = CreateEvent(NULL, FALSE, FALSE, NULL); - infoPtr->writeThreadExiting = FALSE; infoPtr->writeThread = CreateThread(NULL, 256, SerialWriterThread, - infoPtr, 0, &id); - /* Wait for thread to initialize (using evStartWriter) */ - WaitForSingleObject(infoPtr->evStartWriter, 5000); - /* Wake-up it to signal we've get an event */ - SetEvent(infoPtr->evStopWriter); + TclPipeThreadCreateTI(&infoPtr->writeTI, infoPtr, + infoPtr->evWritable), 0, NULL); } /* -- cgit v0.12