From 306e3ea3e928c4b6fbe2bc37ad685d182b2a059e Mon Sep 17 00:00:00 2001 From: andreas_kupries Date: Wed, 4 Aug 2010 16:47:15 +0000 Subject: * generic/tclIORChan.c: [Bug 3034840]: Fixed reference counting * tests/ioCmd.test: in InvokeTclMethod and callers. --- ChangeLog | 7 +++-- generic/tclIORChan.c | 87 +++++++++++++++++++++++++++++++++++++--------------- 2 files changed, 68 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index f43e6cf..6ede49d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,10 +1,13 @@ +2010-08-04 Andreas Kupries + + * generic/tclIORChan.c: [Bug 3034840]: Fixed reference counting + * tests/ioCmd.test: in InvokeTclMethod and callers. + 2010-08-03 Andreas Kupries * tests/var.test (var-19.1): [Bug 3037525]: Added test demonstrating the local hashtable deletion crash and fix. -2010-08-03 Andreas Kupries - * tests/info.test (info-39.1, test_info_frame): Changed absolute to relative frame adressing to handle difference between testing with -singleproc 1 vs. the default -singleproc 0. Plus comment diff --git a/generic/tclIORChan.c b/generic/tclIORChan.c index 03ddc16..c88629f 100644 --- a/generic/tclIORChan.c +++ b/generic/tclIORChan.c @@ -15,7 +15,7 @@ * See the file "license.terms" for information on usage and redistribution of * this file, and for a DISCLAIMER OF ALL WARRANTIES. * - * RCS: @(#) $Id: tclIORChan.c,v 1.28.2.10 2010/03/30 21:17:30 andreas_kupries Exp $ + * RCS: @(#) $Id: tclIORChan.c,v 1.28.2.11 2010/08/04 16:47:15 andreas_kupries Exp $ */ #include @@ -586,6 +586,7 @@ TclChanCreateObjCmd( */ modeObj = DecodeEventMask(mode); + /* assert modeObj.refCount == 1 */ result = InvokeTclMethod(rcPtr, "initialize", modeObj, NULL, &resObj); Tcl_DecrRefCount(modeObj); if (result != TCL_OK) { @@ -1241,6 +1242,8 @@ ReflectInput( Tcl_Preserve(rcPtr); toReadObj = Tcl_NewIntObj(toRead); + Tcl_IncrRefCount(toReadObj); + if (InvokeTclMethod(rcPtr, "read", toReadObj, NULL, &resObj)!=TCL_OK) { int code = ErrnoReturn (rcPtr, resObj); @@ -1267,6 +1270,7 @@ ReflectInput( } stop: + Tcl_DecrRefCount(toReadObj); Tcl_DecrRefCount(resObj); /* Remove reference held from invoke */ Tcl_Release(rcPtr); return bytec; @@ -1353,6 +1357,8 @@ ReflectOutput( Tcl_Preserve(rcPtr); bufObj = Tcl_NewByteArrayObj((unsigned char *) buf, toWrite); + Tcl_IncrRefCount(bufObj); + if (InvokeTclMethod(rcPtr, "write", bufObj, NULL, &resObj) != TCL_OK) { int code = ErrnoReturn(rcPtr, resObj); @@ -1392,6 +1398,7 @@ ReflectOutput( *errorCodePtr = EOK; stop: + Tcl_DecrRefCount(bufObj); Tcl_DecrRefCount(resObj); /* Remove reference held from invoke */ Tcl_Release(rcPtr); return written; @@ -1462,6 +1469,9 @@ ReflectSeekWide( offObj = Tcl_NewWideIntObj(offset); baseObj = Tcl_NewStringObj((seekMode == SEEK_SET) ? "start" : ((seekMode == SEEK_CUR) ? "current" : "end"), -1); + Tcl_IncrRefCount(offObj); + Tcl_IncrRefCount(baseObj); + if (InvokeTclMethod(rcPtr, "seek", offObj, baseObj, &resObj)!=TCL_OK) { Tcl_SetChannelError(rcPtr->chan, resObj); goto invalid; @@ -1479,6 +1489,8 @@ ReflectSeekWide( *errorCodePtr = EOK; stop: + Tcl_DecrRefCount(offObj); + Tcl_DecrRefCount(baseObj); Tcl_DecrRefCount(resObj); /* Remove reference held from invoke */ Tcl_Release(rcPtr); return newLoc; @@ -1574,6 +1586,7 @@ ReflectWatch( Tcl_Preserve(rcPtr); maskObj = DecodeEventMask(mask); + /* assert maskObj.refCount == 1 */ (void) InvokeTclMethod(rcPtr, "watch", maskObj, NULL, NULL); Tcl_DecrRefCount(maskObj); @@ -1629,6 +1642,7 @@ ReflectBlock( #endif blockObj = Tcl_NewBooleanObj(!nonblocking); + Tcl_IncrRefCount(blockObj); Tcl_Preserve(rcPtr); @@ -1639,6 +1653,7 @@ ReflectBlock( errorNum = EOK; } + Tcl_DecrRefCount(blockObj); Tcl_DecrRefCount(resObj); /* Remove reference held from invoke */ Tcl_Release(rcPtr); @@ -1701,11 +1716,17 @@ ReflectSetOption( optionObj = Tcl_NewStringObj(optionName, -1); valueObj = Tcl_NewStringObj(newValue, -1); + + Tcl_IncrRefCount(optionObj); + Tcl_IncrRefCount(valueObj); + result = InvokeTclMethod(rcPtr, "configure",optionObj,valueObj, &resObj); if (result != TCL_OK) { UnmarshallErrorResult(interp, resObj); } + Tcl_DecrRefCount(optionObj); + Tcl_DecrRefCount(valueObj); Tcl_DecrRefCount(resObj); /* Remove reference held from invoke */ Tcl_Release(rcPtr); return result; @@ -1742,7 +1763,7 @@ ReflectGetOption( ReflectedChannel *rcPtr = (ReflectedChannel*) clientData; Tcl_Obj *optionObj; Tcl_Obj *resObj; /* Result data for 'configure' */ - int listc; + int listc, result = TCL_OK; Tcl_Obj **listv; const char *method; @@ -1792,6 +1813,7 @@ ReflectGetOption( method = "cget"; optionObj = Tcl_NewStringObj(optionName, -1); + Tcl_IncrRefCount(optionObj); } Tcl_Preserve(rcPtr); @@ -1849,13 +1871,17 @@ ReflectGetOption( } ok: + result = TCL_OK; + stop: + if (optionObj) { + Tcl_DecrRefCount(optionObj); + } Tcl_DecrRefCount(resObj); /* Remove reference held from invoke */ Tcl_Release(rcPtr); - return TCL_OK; + return result; error: - Tcl_DecrRefCount(resObj); /* Remove reference held from invoke */ - Tcl_Release(rcPtr); - return TCL_ERROR; + result = TCL_ERROR; + goto stop; } /* @@ -2153,6 +2179,11 @@ FreeReflectedChannel( * Side effects: * Arbitrary, as it calls upon a Tcl script. * + * Contract: + * argOneObj.refCount >= 1 on entry and exit, if argOneObj != NULL + * argTwoObj.refCount >= 1 on entry and exit, if argTwoObj != NULL + * resObj.refCount in {0, 1, ...} + * *---------------------------------------------------------------------- */ @@ -2182,16 +2213,10 @@ InvokeTclMethod( Tcl_IncrRefCount(resObj); } - /* - * Cleanup of the dynamic parts of the command. - */ - - if (argOneObj) { - Tcl_DecrRefCount(argOneObj); - if (argTwoObj) { - Tcl_DecrRefCount(argTwoObj); - } - } + /* + * Not touching argOneObj, argTwoObj, they have not been used. + * See the contract as well. + */ return TCL_ERROR; } @@ -2218,11 +2243,9 @@ InvokeTclMethod( cmdc = rcPtr->argc; if (argOneObj) { - Tcl_IncrRefCount(argOneObj); rcPtr->argv[cmdc] = argOneObj; cmdc++; if (argTwoObj) { - Tcl_IncrRefCount(argTwoObj); rcPtr->argv[cmdc] = argTwoObj; cmdc++; } @@ -2285,15 +2308,13 @@ InvokeTclMethod( /* * Cleanup of the dynamic parts of the command. + * + * The detail objects survived the Tcl_EvalObjv without change because of + * the contract. Therefore there is no need to decrement the refcounts. Only + * the internal method object has to be disposed of. */ Tcl_DecrRefCount(methObj); - if (argOneObj) { - Tcl_DecrRefCount(argOneObj); - if (argTwoObj) { - Tcl_DecrRefCount(argTwoObj); - } - } /* * The resObj has a ref count of 1 at this location. This means that the @@ -2844,6 +2865,7 @@ ForwardProc( case ForwardedInput: { Tcl_Obj *toReadObj = Tcl_NewIntObj(paramPtr->input.toRead); + Tcl_IncrRefCount(toReadObj); Tcl_Preserve(rcPtr); if (InvokeTclMethod(rcPtr, "read", toReadObj, NULL, &resObj)!=TCL_OK){ @@ -2876,12 +2898,14 @@ ForwardProc( } } Tcl_Release(rcPtr); + Tcl_DecrRefCount(toReadObj); break; } case ForwardedOutput: { Tcl_Obj *bufObj = Tcl_NewByteArrayObj((unsigned char *) paramPtr->output.buf, paramPtr->output.toWrite); + Tcl_IncrRefCount(bufObj); Tcl_Preserve(rcPtr); if (InvokeTclMethod(rcPtr, "write", bufObj, NULL, &resObj) != TCL_OK) { @@ -2911,6 +2935,7 @@ ForwardProc( } } Tcl_Release(rcPtr); + Tcl_DecrRefCount(bufObj); break; } @@ -2920,6 +2945,9 @@ ForwardProc( (paramPtr->seek.seekMode==SEEK_SET) ? "start" : (paramPtr->seek.seekMode==SEEK_CUR) ? "current" : "end", -1); + Tcl_IncrRefCount(offObj); + Tcl_IncrRefCount(baseObj); + Tcl_Preserve(rcPtr); if (InvokeTclMethod(rcPtr, "seek", offObj, baseObj, &resObj)!=TCL_OK){ ForwardSetObjError(paramPtr, resObj); @@ -2945,11 +2973,14 @@ ForwardProc( } } Tcl_Release(rcPtr); + Tcl_DecrRefCount(offObj); + Tcl_DecrRefCount(baseObj); break; } case ForwardedWatch: { Tcl_Obj *maskObj = DecodeEventMask(paramPtr->watch.mask); + /* assert maskObj.refCount == 1 */ Tcl_Preserve(rcPtr); (void) InvokeTclMethod(rcPtr, "watch", maskObj, NULL, NULL); @@ -2960,6 +2991,7 @@ ForwardProc( case ForwardedBlock: { Tcl_Obj *blockObj = Tcl_NewBooleanObj(!paramPtr->block.nonblocking); + Tcl_IncrRefCount(blockObj); Tcl_Preserve(rcPtr); if (InvokeTclMethod(rcPtr, "blocking", blockObj, NULL, @@ -2967,6 +2999,7 @@ ForwardProc( ForwardSetObjError(paramPtr, resObj); } Tcl_Release(rcPtr); + Tcl_DecrRefCount(blockObj); break; } @@ -2974,12 +3007,16 @@ ForwardProc( Tcl_Obj *optionObj = Tcl_NewStringObj(paramPtr->setOpt.name, -1); Tcl_Obj *valueObj = Tcl_NewStringObj(paramPtr->setOpt.value, -1); + Tcl_IncrRefCount(optionObj); + Tcl_IncrRefCount(valueObj); Tcl_Preserve(rcPtr); if (InvokeTclMethod(rcPtr, "configure", optionObj, valueObj, &resObj) != TCL_OK) { ForwardSetObjError(paramPtr, resObj); } Tcl_Release(rcPtr); + Tcl_DecrRefCount(optionObj); + Tcl_DecrRefCount(valueObj); break; } @@ -2989,6 +3026,7 @@ ForwardProc( */ Tcl_Obj *optionObj = Tcl_NewStringObj(paramPtr->getOpt.name, -1); + Tcl_IncrRefCount(optionObj); Tcl_Preserve(rcPtr); if (InvokeTclMethod(rcPtr, "cget", optionObj, NULL, &resObj)!=TCL_OK){ @@ -2998,6 +3036,7 @@ ForwardProc( TclGetString(resObj), -1); } Tcl_Release(rcPtr); + Tcl_DecrRefCount(optionObj); break; } -- cgit v0.12