diff options
| -rw-r--r-- | ChangeLog | 20 | ||||
| -rw-r--r-- | generic/tclIORTrans.c | 68 |
2 files changed, 56 insertions, 32 deletions
@@ -1,14 +1,20 @@ -2011-09-15 Don Porter <dgp@users.sourceforge.net> +2011-09-20 Don Porter <dgp@users.sourceforge.net> + + * generic/tclIORTrans.c: Revised ReflectClose() and + FreeReflectedTransform() so that we stop leaking ReflectedTransforms, + yet free all Tcl_Obj values in the same thread that alloced them. + +2011-09-19 Don Porter <dgp@users.sourceforge.net> * tests/ioTrans.test: Conversion from [testthread] to Thread package stops most memory leaks. - * tests/thread.test: Plug most memory leaks in thread.test Constrain - the rest to be skipped during `make valgrind`. Tests using the - [testthread cancel] testing command are leaky. Corrections wait for - either addition of [thread::cancel] to the Thread package, or improvements - to the [testthread] testing command to make leak-free versions of these - tests possible. + * tests/thread.test: Plug most memory leaks in thread.test. + Constrain the rest to be skipped during `make valgrind`. Tests using + the [testthread cancel] testing command are leaky. Corrections wait + for either addition of [thread::cancel] to the Thread package, or + improvements to the [testthread] testing command to make leak-free + versions of these tests possible. * generic/tclIORChan.c: Plug all memory leaks in ioCmd.test exposed * tests/ioCmd.test: by `make valgrind`. diff --git a/generic/tclIORTrans.c b/generic/tclIORTrans.c index fa973c7..ef37d5c 100644 --- a/generic/tclIORTrans.c +++ b/generic/tclIORTrans.c @@ -407,6 +407,7 @@ static ReflectedTransform * NewReflectedTransform(Tcl_Interp *interp, Tcl_Channel parentChan); static Tcl_Obj * NextHandle(void); static void FreeReflectedTransform(ReflectedTransform *rtPtr); +static void FreeReflectedTransformArgs(ReflectedTransform *rtPtr); static int InvokeTclMethod(ReflectedTransform *rtPtr, const char *method, Tcl_Obj *argOneObj, Tcl_Obj *argTwoObj, Tcl_Obj **resultObjPtr); @@ -881,6 +882,7 @@ ReflectClose( Tcl_Interp *interp) { ReflectedTransform *rtPtr = clientData; + int errorCode, errorCodeSet = 0; int result; /* Result code for 'close' */ Tcl_Obj *resObj; /* Result data for 'close' */ ReflectedTransformMap *rtmPtr; @@ -912,15 +914,9 @@ ReflectClose( ForwardOpToOwnerThread(rtPtr, ForwardedClose, &p); result = p.base.code; - /* - * FreeReflectedTransform is done in the forwarded operation!, in - * the other thread. rtPtr here is gone! - */ - if (result != TCL_OK) { FreeReceivedError(&p); } - return EOK; } #endif @@ -937,20 +933,30 @@ ReflectClose( */ if (HAS(rtPtr->methods, METH_DRAIN) && !rtPtr->readIsDrained) { - int errorCode; - if (!TransformDrain(rtPtr, &errorCode)) { - Tcl_EventuallyFree (rtPtr, (Tcl_FreeProc *) FreeReflectedTransform); - return errorCode; +#ifdef TCL_THREADS + if (rtPtr->thread != Tcl_GetCurrentThread()) { + Tcl_EventuallyFree (rtPtr, + (Tcl_FreeProc *) FreeReflectedTransform); + return errorCode; + } +#endif + errorCodeSet = 1; + goto cleanup; } } if (HAS(rtPtr->methods, METH_FLUSH)) { - int errorCode; - if (!TransformFlush(rtPtr, &errorCode, FLUSH_WRITE)) { - Tcl_EventuallyFree (rtPtr, (Tcl_FreeProc *) FreeReflectedTransform); - return errorCode; +#ifdef TCL_THREADS + if (rtPtr->thread != Tcl_GetCurrentThread()) { + Tcl_EventuallyFree (rtPtr, + (Tcl_FreeProc *) FreeReflectedTransform); + return errorCode; + } +#endif + errorCodeSet = 1; + goto cleanup; } } @@ -965,10 +971,7 @@ ReflectClose( ForwardOpToOwnerThread(rtPtr, ForwardedClose, &p); result = p.base.code; - /* - * FreeReflectedTransform is done in the forwarded operation!, in the - * other thread. rtPtr here is gone! - */ + Tcl_EventuallyFree (rtPtr, (Tcl_FreeProc *) FreeReflectedTransform); if (result != TCL_OK) { PassReceivedErrorInterp(interp, &p); @@ -990,6 +993,8 @@ ReflectClose( Tcl_DecrRefCount(resObj); /* Remove reference we held from the * invoke. */ + cleanup: + /* * Remove the transform from the map before releasing the memory, to * prevent future accesses from finding and dereferencing a dangling @@ -1026,7 +1031,7 @@ ReflectClose( #endif Tcl_EventuallyFree (rtPtr, (Tcl_FreeProc *) FreeReflectedTransform); - return (result == TCL_OK) ? EOK : EINVAL; + return errorCodeSet ? errorCode : ((result == TCL_OK) ? EOK : EINVAL); } /* @@ -1866,18 +1871,18 @@ NextHandle(void) } static void -FreeReflectedTransform( +FreeReflectedTransformArgs( ReflectedTransform *rtPtr) { - int i, n; + int i, n = rtPtr->argc - 2; - TimerKill(rtPtr); - ResultClear(&rtPtr->result); + if (n < 0) { + return; + } Tcl_DecrRefCount(rtPtr->handle); rtPtr->handle = NULL; - n = rtPtr->argc - 2; for (i=0; i<n; i++) { Tcl_DecrRefCount(rtPtr->argv[i]); } @@ -1888,6 +1893,18 @@ FreeReflectedTransform( */ Tcl_DecrRefCount(rtPtr->argv[n+1]); + rtPtr->argc = 1; +} + +static void +FreeReflectedTransform( + ReflectedTransform *rtPtr) +{ + TimerKill(rtPtr); + ResultClear(&rtPtr->result); + + FreeReflectedTransformArgs(rtPtr); + ckfree(rtPtr->argv); ckfree(rtPtr); } @@ -2337,6 +2354,7 @@ DeleteThreadReflectedTransformMap( ReflectedTransform *rtPtr = Tcl_GetHashValue(hPtr); rtPtr->interp = NULL; + FreeReflectedTransformArgs(rtPtr); Tcl_DeleteHashEntry(hPtr); } ckfree(rtmPtr); @@ -2541,7 +2559,7 @@ ForwardProc( hPtr = Tcl_FindHashEntry(&rtmPtr->map, Tcl_GetString(rtPtr->handle)); Tcl_DeleteHashEntry(hPtr); - Tcl_EventuallyFree (rtPtr, (Tcl_FreeProc *) FreeReflectedTransform); + FreeReflectedTransformArgs(rtPtr); break; case ForwardedInput: { |