summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorsebres <sebres@users.sourceforge.net>2017-04-11 18:08:43 (GMT)
committersebres <sebres@users.sourceforge.net>2017-04-11 18:08:43 (GMT)
commit1931e1e055c94f3b332719d70e97ab82f801e5d7 (patch)
treec5c45f3d46c115b11545370bd04ef7834fe6330a
parentd99c5f05d972536e1110de39d62d9d167524f6a2 (diff)
downloadtcl-1931e1e055c94f3b332719d70e97ab82f801e5d7.zip
tcl-1931e1e055c94f3b332719d70e97ab82f801e5d7.tar.gz
tcl-1931e1e055c94f3b332719d70e97ab82f801e5d7.tar.bz2
code review and fix small memory leak using ckalloc, without finalization of tcl subsystem in the worker (if it owns TI structure and calls ckfree)
-rw-r--r--win/tclWinPipe.c107
1 files changed, 71 insertions, 36 deletions
diff --git a/win/tclWinPipe.c b/win/tclWinPipe.c
index bb76eef..13d90b6 100644
--- a/win/tclWinPipe.c
+++ b/win/tclWinPipe.c
@@ -101,11 +101,42 @@ typedef struct PipeThreadInfo {
} PipeThreadInfo;
-#define PTI_STATE_IDLE 0
-#define PTI_STATE_WORK 1
-#define PTI_STATE_STOP 2
-#define PTI_STATE_END 4
-#define PTI_STATE_DOWN 8
+/* If pipe-workers will use some tcl subsystem, we can use ckalloc without
+ * more overhead for finalize thread (should be executed anyway)
+ *
+ * #define _PTI_USE_CKALLOC 1
+ */
+
+/*
+ * State of the pipe-worker.
+ *
+ * State PTI_STATE_STOP possible from idle state only, worker owns TI structure.
+ * Otherwise PTI_STATE_END used (main thread hold ownership of the TI).
+ */
+
+#define PTI_STATE_IDLE 0 /* idle or not yet initialzed */
+#define PTI_STATE_WORK 1 /* in work */
+#define PTI_STATE_STOP 2 /* thread should stop work (owns TI structure) */
+#define PTI_STATE_END 4 /* thread should stop work (worker is busy) */
+#define PTI_STATE_DOWN 8 /* worker is down */
+
+
+PipeThreadInfo *
+PipeThreadCreateTI(
+ PipeThreadInfo **pipeTIPtr,
+ ClientData clientData)
+{
+ PipeThreadInfo *pipeTI;
+#ifndef _PTI_USE_CKALLOC
+ pipeTI = malloc(sizeof(PipeThreadInfo));
+#else
+ pipeTI = ckalloc(sizeof(PipeThreadInfo));
+#endif
+ pipeTI->evControl = CreateEvent(NULL, FALSE, FALSE, NULL);
+ pipeTI->state = PTI_STATE_IDLE;
+ pipeTI->clientData = clientData;
+ return (*pipeTIPtr = pipeTI);
+}
int
PipeThreadWaitForSignal(
@@ -292,7 +323,7 @@ PipeThreadStop(
/* if we want TIP#398-fast-exit. */
if (WaitForSingleObject(hThread,
- TclInExit() ? 0 : 0) == WAIT_TIMEOUT) {
+ TclInExit() ? 0 : 20) == WAIT_TIMEOUT) {
/*
* The thread must be blocked waiting for the pipe to
@@ -343,7 +374,11 @@ PipeThreadStop(
*pipeTIPtr = NULL;
if (pipeTI) {
CloseHandle(pipeTI->evControl);
+ #ifndef _PTI_USE_CKALLOC
+ free(pipeTI);
+ #else
ckfree(pipeTI);
+ #endif
}
}
@@ -365,7 +400,13 @@ PipeThreadExit(
if ((state = InterlockedExchange(&pipeTI->state,
PTI_STATE_DOWN)) == PTI_STATE_STOP) {
CloseHandle(pipeTI->evControl);
+ #ifndef _PTI_USE_CKALLOC
+ free(pipeTI);
+ #else
ckfree(pipeTI);
+ /* be sure all subsystems used are finalized */
+ Tcl_FinalizeThread();
+ #endif
}
}
@@ -1869,14 +1910,9 @@ TclpCreateCommandChannel(
* Start the background reader thread.
*/
- PipeThreadInfo *pipeTI = ckalloc(sizeof(PipeThreadInfo));
- pipeTI->evControl = CreateEvent(NULL, FALSE, FALSE, NULL);
- pipeTI->state = PTI_STATE_IDLE;
- pipeTI->clientData = infoPtr;
- infoPtr->readTI = pipeTI;
infoPtr->readable = CreateEvent(NULL, TRUE, TRUE, NULL);
infoPtr->readThread = CreateThread(NULL, 256, PipeReaderThread,
- pipeTI, 0, &id);
+ PipeThreadCreateTI(&infoPtr->readTI, infoPtr), 0, &id);
SetThreadPriority(infoPtr->readThread, THREAD_PRIORITY_HIGHEST);
infoPtr->validMask |= TCL_READABLE;
} else {
@@ -1888,14 +1924,9 @@ TclpCreateCommandChannel(
* Start the background writer thread.
*/
- PipeThreadInfo *pipeTI = ckalloc(sizeof(PipeThreadInfo));
- pipeTI->evControl = CreateEvent(NULL, FALSE, FALSE, NULL);
- pipeTI->state = PTI_STATE_IDLE;
- pipeTI->clientData = infoPtr;
- infoPtr->writeTI = pipeTI;
infoPtr->writable = CreateEvent(NULL, TRUE, TRUE, NULL);
infoPtr->writeThread = CreateThread(NULL, 256, PipeWriterThread,
- pipeTI, 0, &id);
+ PipeThreadCreateTI(&infoPtr->writeTI, infoPtr), 0, &id);
SetThreadPriority(infoPtr->writeThread, THREAD_PRIORITY_HIGHEST);
infoPtr->validMask |= TCL_WRITABLE;
} else {
@@ -2114,27 +2145,27 @@ PipeClose2Proc(
&& (pipePtr->writeFile != NULL)) {
if (pipePtr->writeThread) {
- /* Notify thread we will stop work and check it still seems to work */
- if (!PipeThreadStopSignal(&pipePtr->writeTI)) {
- /*
- * Wait for the writer thread to finish the current buffer, then
- * terminate the thread and close the handles. If the channel is
- * 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()) {
+ /*
+ * Wait for the writer thread to finish the current buffer, then
+ * terminate the thread and close the handles. If the channel is
+ * 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()) {
- /* give it a chance to leave honorably */
- if (WaitForSingleObject(pipePtr->writable, 0) == WAIT_TIMEOUT) {
- return EWOULDBLOCK;
- }
+ /* give it a chance to leave honorably */
+ PipeThreadStopSignal(&pipePtr->writeTI);
- } else {
+ if (WaitForSingleObject(pipePtr->writable, 20) == WAIT_TIMEOUT) {
+ return EWOULDBLOCK;
+ }
- WaitForSingleObject(pipePtr->writable, INFINITE);
+ } else {
+
+ WaitForSingleObject(pipePtr->writable, INFINITE);
- }
}
+
PipeThreadStop(&pipePtr->writeTI, pipePtr->writeThread);
CloseHandle(pipePtr->writable);
@@ -3128,7 +3159,7 @@ PipeWriterThread(
{
PipeThreadInfo *pipeTI = (PipeThreadInfo *)arg;
PipeInfo *infoPtr = NULL; /* access info only after success init/wait */
- HANDLE handle = NULL;
+ HANDLE handle = NULL, writable = NULL;
DWORD count, toWrite;
char *buf;
int done = 0;
@@ -3139,12 +3170,16 @@ PipeWriterThread(
*/
if (!PipeThreadWaitForSignal(&pipeTI)) {
/* exit */
+ if (writable) {
+ SetEvent(writable);
+ }
break;
}
if (!infoPtr) {
infoPtr = (PipeInfo *)pipeTI->clientData;
handle = ((WinFile *) infoPtr->writeFile)->handle;
+ writable = infoPtr->writable;
}
buf = infoPtr->writeBuf;
@@ -3170,7 +3205,7 @@ PipeWriterThread(
* waking up the notifier thread.
*/
- SetEvent(infoPtr->writable);
+ SetEvent(writable);
/*
* Alert the foreground thread. Note that we need to treat this like a