Index: TODO =================================================================== diff -u -r3ea11774a1d822bb63611f31ec092db6b4853c4e -r3cd976554a5ed01b8f1ae6aa66865aee1dffa82f --- TODO (.../TODO) (revision 3ea11774a1d822bb63611f31ec092db6b4853c4e) +++ TODO (.../TODO) (revision 3cd976554a5ed01b8f1ae6aa66865aee1dffa82f) @@ -3100,6 +3100,14 @@ * New function DeleteProcsAndVars() to trigger deletion of ParamDefs (fixes a small memory leak); * improved comments + * improved INCR_REF_COUNT/DECR_REF_COUNT for easier + tracking of potential refcount errors + * added macros DECR_REF_COUNT2() and INCR_REF_COUNT2() + for easing the association of refcounts to locations + in the code by providing names for refcounts. + * fixed a refcount bug for valueObjs in non-NRE-enabled + versions in the argument vector of scripted methods + (found via INCR_REF_COUNT2/DECR_REF_COUNT2) Index: generic/nsf.c =================================================================== diff -u -r3ea11774a1d822bb63611f31ec092db6b4853c4e -r3cd976554a5ed01b8f1ae6aa66865aee1dffa82f --- generic/nsf.c (.../nsf.c) (revision 3ea11774a1d822bb63611f31ec092db6b4853c4e) +++ generic/nsf.c (.../nsf.c) (revision 3cd976554a5ed01b8f1ae6aa66865aee1dffa82f) @@ -433,7 +433,7 @@ * * Initialize a ParseContext with default values and allocate * memory if needed. Every ParseContext has to be initialized - * before usage and has to be freed with ParseContextRelease. + * before usage and has to be freed with ParseContextRelease(). * * Results: * None. @@ -550,10 +550,11 @@ int i; for (i = 0; i < pcPtr->objc-1; i++) { /*fprintf(stderr, "ParseContextRelease %p check [%d] obj %p refCount %d (%s)\n", - pcPtr, i, pcPtr->objv[i], pcPtr->objv[i]->refCount, ObjStr(pcPtr->objv[i]));*/ + pcPtr, i, pcPtr->objv[i], pcPtr->objv[i]->refCount, + ObjStr(pcPtr->objv[i]));*/ if (pcPtr->flags[i] & NSF_PC_MUST_DECR) { assert(pcPtr->objv[i]->refCount > 0); - DECR_REF_COUNT(pcPtr->objv[i]); + DECR_REF_COUNT2("valueObj", pcPtr->objv[i]); } } } @@ -811,7 +812,7 @@ int result; ALLOC_ON_STACK(Tcl_Obj *, objc+2, ov); - INCR_REF_COUNT(nameObj); + INCR_REF_COUNT2("nameObj", nameObj); ov[0] = NULL; ov[1] = nameObj; @@ -821,7 +822,7 @@ result = NsfCCreateMethod(interp, cl, ObjStr(nameObj), objc+2, ov); FREE_ON_STACK(Tcl_Obj*, ov); - DECR_REF_COUNT(nameObj); + DECR_REF_COUNT2("nameObj", nameObj); return result; } @@ -2300,11 +2301,11 @@ static void ObjectSystemFree(Tcl_Interp *interp, NsfObjectSystem *osPtr) { - int i; + int idx; - for (i=0; i<=NSF_o_unknown_idx; i++) { - if (osPtr->methods[i]) { DECR_REF_COUNT(osPtr->methods[i]); } - if (osPtr->handles[i]) { DECR_REF_COUNT(osPtr->handles[i]); } + for (idx=0; idx<=NSF_o_unknown_idx; idx++) { + if (osPtr->methods[idx]) { DECR_REF_COUNT(osPtr->methods[idx]); } + if (osPtr->handles[idx]) { DECR_REF_COUNT(osPtr->handles[idx]); } } if (osPtr->rootMetaClass && osPtr->rootClass) { @@ -2922,12 +2923,12 @@ * *********************************************************/ -typedef struct nsfResolvedVarInfo { +typedef struct NsfResolvedVarInfo { Tcl_ResolvedVarInfo vInfo; /* This must be the first element. */ NsfObject *lastObject; Tcl_Var var; Tcl_Obj *nameObj; -} nsfResolvedVarInfo; +} NsfResolvedVarInfo; /* *---------------------------------------------------------------------- @@ -2982,7 +2983,7 @@ static Tcl_Var CompiledColonVarFetch(Tcl_Interp *interp, Tcl_ResolvedVarInfo *vinfoPtr) { - nsfResolvedVarInfo *resVarInfo = (nsfResolvedVarInfo *)vinfoPtr; + NsfResolvedVarInfo *resVarInfo = (NsfResolvedVarInfo *)vinfoPtr; NsfCallStackContent *cscPtr = CallStackGetTopFrame0(interp); NsfObject *object = cscPtr ? cscPtr->self : NULL; TclVarHashTable *varTablePtr; @@ -3078,11 +3079,11 @@ */ static void CompiledColonVarFree(Tcl_ResolvedVarInfo *vInfoPtr) { - nsfResolvedVarInfo *resVarInfo = (nsfResolvedVarInfo *)vInfoPtr; + NsfResolvedVarInfo *resVarInfo = (NsfResolvedVarInfo *)vInfoPtr; DECR_REF_COUNT(resVarInfo->nameObj); if (resVarInfo->var) {HashVarFree(resVarInfo->var);} - ckfree((char *) vInfoPtr); + FREE(NsfResolvedVarInfo, vInfoPtr); } /* @@ -3131,15 +3132,15 @@ #endif if (object && FOR_COLON_RESOLVER(name)) { - nsfResolvedVarInfo *vInfoPtr = (nsfResolvedVarInfo *) ckalloc(sizeof(nsfResolvedVarInfo)); + NsfResolvedVarInfo *resVarInfo = NEW(NsfResolvedVarInfo); - vInfoPtr->vInfo.fetchProc = CompiledColonVarFetch; - vInfoPtr->vInfo.deleteProc = CompiledColonVarFree; /* if NULL, tcl does a ckfree on proc clean up */ - vInfoPtr->lastObject = NULL; - vInfoPtr->var = NULL; - vInfoPtr->nameObj = Tcl_NewStringObj(name+1, length-1); - INCR_REF_COUNT(vInfoPtr->nameObj); - *rPtr = (Tcl_ResolvedVarInfo *)vInfoPtr; + resVarInfo->vInfo.fetchProc = CompiledColonVarFetch; + resVarInfo->vInfo.deleteProc = CompiledColonVarFree; /* if NULL, tcl does a ckfree on proc clean up */ + resVarInfo->lastObject = NULL; + resVarInfo->var = NULL; + resVarInfo->nameObj = Tcl_NewStringObj(name+1, length-1); + INCR_REF_COUNT(resVarInfo->nameObj); + *rPtr = (Tcl_ResolvedVarInfo *)resVarInfo; return TCL_OK; } @@ -4353,7 +4354,7 @@ Tcl_UnsetVar2(interp, NsfGlobalStrings[NSF_AUTONAMES], ObjStr(nameObj), flgs); } resultObj = NsfGlobalObjs[NSF_EMPTY]; - INCR_REF_COUNT(resultObj); + INCR_REF_COUNT2("autoname", resultObj); } else { int mustCopy = 1, format = 0; @@ -4369,14 +4370,14 @@ if (isupper((int)firstChar)) { buffer[0] = tolower((int)firstChar); resultObj = Tcl_NewStringObj(buffer, 1); - INCR_REF_COUNT(resultObj); + INCR_REF_COUNT2("autoname", resultObj); Tcl_AppendLimitedToObj(resultObj, nextChars, -1, INT_MAX, NULL); mustCopy = 0; } } if (mustCopy) { resultObj = Tcl_DuplicateObj(nameObj); - INCR_REF_COUNT(resultObj); + INCR_REF_COUNT2("autoname", resultObj); /* fprintf(stderr, "*** copy %p %s = %p\n", name, ObjStr(name), resultObj); */ @@ -4412,7 +4413,7 @@ } DECR_REF_COUNT(resultObj); resultObj = Tcl_DuplicateObj(Tcl_GetObjResult(interp)); - INCR_REF_COUNT(resultObj); + INCR_REF_COUNT2("autoname", resultObj); Tcl_SetObjResult(interp, savedResultObj); DECR_REF_COUNT(savedResultObj); FREE_ON_STACK(Tcl_Obj*, ov); @@ -8082,19 +8083,17 @@ */ static int ProcMethodDispatchFinalize(ClientData data[], Tcl_Interp *interp, int result) { -#if defined(NRE) ParseContext *pcPtr = data[0]; -#endif NsfCallStackContent *cscPtr = data[1]; /*CONST char *methodName = data[2];*/ #if defined(NSF_WITH_ASSERTIONS) NsfObject *object = cscPtr->self; NsfObjectOpt *opt = object->opt; #endif - /*fprintf(stderr, "ProcMethodDispatchFinalize %s flags %.6x isNRE %d\n", + /*fprintf(stderr, "ProcMethodDispatchFinalize %s flags %.6x isNRE %d pcPtr %p\n", ObjectName(object), - cscPtr->flags, (cscPtr->flags & NSF_CSC_CALL_IS_NRE));*/ + cscPtr->flags, (cscPtr->flags & NSF_CSC_CALL_IS_NRE), pcPtr);*/ #if defined(NSF_WITH_ASSERTIONS) if (opt && object->teardown && (opt->checkoptions & CHECK_POST)) { @@ -8121,6 +8120,10 @@ CscFinish(interp, cscPtr, result, "scripted finalize"); } +#else + if (pcPtr) { + ParseContextRelease(pcPtr); + } #endif return result; @@ -8159,7 +8162,7 @@ # endif ParseContextRelease(pcPtr); - NsfTclStackFree(interp, pcPtr, "proc dispatch finialize release parse context"); + NsfTclStackFree(interp, pcPtr, "nsf::proc dispatch finialize release parse context"); return result; } @@ -9421,11 +9424,27 @@ * Proc-Creation */ +/* + *---------------------------------------------------------------------- + * AddPrefixToBody -- + * + * Create a fresh TclObj* containing the body with an potential prefix. + * The caller has to decrement the refcount on this Tcl_Obj*. + * + * Results: + * Tcl_Obj + * + * Side effects: + * None. + * + *---------------------------------------------------------------------- + */ + static Tcl_Obj * AddPrefixToBody(Tcl_Obj *body, int paramDefs, NsfParsedParam *paramPtr) { Tcl_Obj *resultBody = Tcl_NewObj(); - INCR_REF_COUNT(resultBody); + INCR_REF_COUNT2("resultBody", resultBody); if (paramDefs && paramPtr->possibleUnknowns > 0) { Tcl_AppendStringsToObj(resultBody, "::nsf::__unset_unknown_args\n", (char *) NULL); @@ -10638,7 +10657,7 @@ NSF_DISALLOWED_ARG_METHOD_PARAMETER, 0, &parsedParam); } - if (result != TCL_OK) { + if (unlikely(result != TCL_OK)) { return result; } @@ -10709,7 +10728,7 @@ if (parsedParam.paramDefs) { DECR_REF_COUNT(ov[2]); } - DECR_REF_COUNT(ov[3]); + DECR_REF_COUNT2("resultBody", ov[3]); return result; } @@ -10810,7 +10829,7 @@ /*fprintf(stderr, "NsfProcStubDeleteProc received %p\n", clientData); fprintf(stderr, "... procName %s paramDefs %p\n", ObjStr(tcd->procName), tcd->paramDefs);*/ - DECR_REF_COUNT(tcd->procName); + DECR_REF_COUNT2("procNameObj",tcd->procName); if (tcd->cmd) { NsfCommandRelease(tcd->cmd); } @@ -11098,7 +11117,7 @@ DStringAppendQualName(dsPtr, cmdNsPtr, Tcl_GetCommandName(interp, cmd)); procNameObj = Tcl_NewStringObj(Tcl_DStringValue(dsPtr), Tcl_DStringLength(dsPtr)); - INCR_REF_COUNT(procNameObj); /* will be freed, when NsfProcStub is deleted */ + INCR_REF_COUNT2("procNameObj", procNameObj); /* will be freed, when NsfProcStub is deleted */ /* * Make sure to create the target namespace under "::nsf::procs::", if @@ -11165,7 +11184,7 @@ result = Tcl_ProcObjCmd(0, interp, 4, ov); DECR_REF_COUNT(argList); - DECR_REF_COUNT(ov[3]); + DECR_REF_COUNT2("resultBody", ov[3]); if (result == TCL_OK) { /* @@ -13678,7 +13697,7 @@ result = SetInstVar(interp, object, objv[0], outObjPtr); if (flags & NSF_PC_MUST_DECR) { - DECR_REF_COUNT(outObjPtr); + DECR_REF_COUNT2("valueObj", outObjPtr); } } return result; @@ -14556,7 +14575,7 @@ NsfPrintError(interp, "invalid value in \"%s\": %s", ObjStr(objPtr), ObjStr(resultObj)); *flags &= ~NSF_PC_MUST_DECR; *outObjPtr = objPtr; - DECR_REF_COUNT(*outObjPtr); + DECR_REF_COUNT2("valueObj", *outObjPtr); DECR_REF_COUNT(resultObj); break; } @@ -14669,9 +14688,10 @@ pcPtr->objv[i] = Tcl_NewBooleanObj(!bool); /* * Perform bookkeeping to avoid that someone releases the new obj - * before we are done. + * before we are done. The according DECR is performed by + * ParseContextRelease() */ - INCR_REF_COUNT(pcPtr->objv[i]); + INCR_REF_COUNT2("valueObj", pcPtr->objv[i]); pcPtr->flags[i] |= NSF_PC_MUST_DECR; pcPtr->status |= NSF_PC_STATUS_MUST_DECR; } @@ -14706,7 +14726,7 @@ newValue, ObjStr(newValue));*/ /* The according DECR is performed by ParseContextRelease() */ - INCR_REF_COUNT(newValue); + INCR_REF_COUNT2("valueObj", newValue); mustDecrNewValue = 1; pcPtr->flags[i] |= NSF_PC_MUST_DECR; pcPtr->status |= NSF_PC_STATUS_MUST_DECR; @@ -14737,7 +14757,7 @@ * here and clear the flag. */ if (mustDecrNewValue) { - DECR_REF_COUNT(newValue); + DECR_REF_COUNT2("valueObj", newValue); pcPtr->flags[i] &= ~NSF_PC_MUST_DECR; } /* @@ -14927,7 +14947,7 @@ if (valueInArgument) { valueObj = Tcl_NewStringObj(valueInArgument+1,-1); - INCR_REF_COUNT(valueObj); + INCR_REF_COUNT2("valueObj", valueObj); pcPtr->flags[j] |= NSF_PC_MUST_DECR; } else { if (nppPtr->converter == Nsf_ConvertToSwitch) { @@ -18846,7 +18866,7 @@ } if (flags & NSF_PC_MUST_DECR) { - DECR_REF_COUNT(outObjPtr); + DECR_REF_COUNT2("valueObj", outObjPtr); } return result; @@ -18870,7 +18890,7 @@ Tcl_Obj *autoname = AutonameIncr(interp, nameObj, object, withInstance, withReset); if (autoname) { Tcl_SetObjResult(interp, autoname); - DECR_REF_COUNT(autoname); + DECR_REF_COUNT2("autoname", autoname); return TCL_OK; } Index: generic/nsf.h =================================================================== diff -u -r6889109b1238e52796b59d0f35b81e00f9f268cf -r3cd976554a5ed01b8f1ae6aa66865aee1dffa82f --- generic/nsf.h (.../nsf.h) (revision 6889109b1238e52796b59d0f35b81e00f9f268cf) +++ generic/nsf.h (.../nsf.h) (revision 3cd976554a5ed01b8f1ae6aa66865aee1dffa82f) @@ -96,7 +96,7 @@ #define NSF_MEM_TRACE 1 #define NSF_MEM_COUNT 1 */ -//#define NSF_MEM_COUNT 1 +#define NSF_MEM_COUNT 1 /* turn tracing output on/off #define NSFOBJ_TRACE 1 Index: generic/nsfInt.h =================================================================== diff -u -r6889109b1238e52796b59d0f35b81e00f9f268cf -r3cd976554a5ed01b8f1ae6aa66865aee1dffa82f --- generic/nsfInt.h (.../nsfInt.h) (revision 6889109b1238e52796b59d0f35b81e00f9f268cf) +++ generic/nsfInt.h (.../nsfInt.h) (revision 3cd976554a5ed01b8f1ae6aa66865aee1dffa82f) @@ -217,7 +217,7 @@ # define NSF_INLINE # define NsfNewObj(A) A=Tcl_NewObj() # define DECR_REF_COUNT(A) \ - MEM_COUNT_FREE("INCR_REF_COUNT",A); Tcl_DecrRefCount(A) + MEM_COUNT_FREE("INCR_REF_COUNT" #A,A); Tcl_DecrRefCount(A) #else /* * This was defined to be inline for anything !sun or __IBMC__ >= 0x0306, @@ -231,12 +231,17 @@ # ifdef USE_TCL_STUBS # define NsfNewObj(A) A=Tcl_NewObj() # define DECR_REF_COUNT(A) \ - MEM_COUNT_FREE("INCR_REF_COUNT",A); assert((A)->refCount > -1); \ + MEM_COUNT_FREE("INCR_REF_COUNT" #A,A); assert((A)->refCount > -1); \ Tcl_DecrRefCount(A) +# define DECR_REF_COUNT2(name,A) \ + MEM_COUNT_FREE("INCR_REF_COUNT-" name,A); assert((A)->refCount > -1); \ + Tcl_DecrRefCount(A) # else # define NsfNewObj(A) TclNewObj(A) # define DECR_REF_COUNT(A) \ - MEM_COUNT_FREE("INCR_REF_COUNT",A); TclDecrRefCount(A) + MEM_COUNT_FREE("INCR_REF_COUNT" #A,A); TclDecrRefCount(A) +# define DECR_REF_COUNT2(name,A) \ + MEM_COUNT_FREE("INCR_REF_COUNT-" name,A); TclDecrRefCount(A) # endif #endif @@ -246,7 +251,8 @@ #define ObjStr(obj) (obj)->bytes ? (obj)->bytes : Tcl_GetString(obj) -#define INCR_REF_COUNT(A) MEM_COUNT_ALLOC("INCR_REF_COUNT",A); Tcl_IncrRefCount(A) +#define INCR_REF_COUNT(A) MEM_COUNT_ALLOC("INCR_REF_COUNT"#A,A); Tcl_IncrRefCount(A) +#define INCR_REF_COUNT2(name,A) MEM_COUNT_ALLOC("INCR_REF_COUNT-" name,A); Tcl_IncrRefCount(A) #ifdef OBJDELETION_TRACE # define PRINTOBJ(ctx,obj) \