Index: TODO =================================================================== diff -u -N -r95c4d29515cfda9283b406ba7a7ac1dd029de8c9 -r3cc93845be8bba214f09b9c6ec29e48de99175ff --- TODO (.../TODO) (revision 95c4d29515cfda9283b406ba7a7ac1dd029de8c9) +++ TODO (.../TODO) (revision 3cc93845be8bba214f09b9c6ec29e48de99175ff) @@ -3475,12 +3475,17 @@ * bring cmds to an alphabetical order (make maintenance easier) * add optical separators between doc items to ease reading +- fixed mem_count on contains.test +- nsf.c: + * minor cleanup + * added regression tests TODO: - private: * document private in tutorial + - add "property" and "attribute" and "info property" and "info slot ..." to migration guide - naming of slot classes * should we switch from "-class" to "-slotclass"? @@ -3498,8 +3503,6 @@ o1 info children ?-type class? ?pattern? - - add "property" and "attribute" and "info property" and "info slot ..." to migration guide - - doc/langRef2.xotcl vs library/xotcl/doc/langRef.xotcl - strange refcounting bug in 8.6b2 bug-is-86.tcl @@ -3526,19 +3529,6 @@ # variable uses nsf::is and attribute uses the slot obj. method variable should # be changed to use the slotobj as well. - - fix mem_count on contains.test (currently 4 / 4) - the minimal script seems to be the following and is related - to the following message - Warning: Have to fix refCount for obj 0x100924210 refCount 2 (name ::nx::Class::slot::__info) - ======================= - package require nx - package forget nx - package req nx - puts stderr ====EXIT - exit - ======================= - - - interface of "variable" and "attribute": * add switch -array for "variable"? (Just setting is trivial, handling setters and incremental setter is more work) Index: generic/nsf.c =================================================================== diff -u -N -rb4266efa283e86989f6f09f3938f41f173580400 -r3cc93845be8bba214f09b9c6ec29e48de99175ff --- generic/nsf.c (.../nsf.c) (revision b4266efa283e86989f6f09f3938f41f173580400) +++ generic/nsf.c (.../nsf.c) (revision 3cc93845be8bba214f09b9c6ec29e48de99175ff) @@ -9070,19 +9070,17 @@ */ if (CmdIsNsfObject(cmd)) { /* - * invoke an aliased object (ensemble object) via method interface + * Invoke an aliased object (ensemble object) via method interface. */ - NsfRuntimeState *rst = RUNTIME_STATE(interp); NsfObject *invokeObj = (NsfObject *)cp; if (invokeObj->flags & NSF_DELETED) { /* - * When we try to call a deleted object, the cmd (alias) is - * automatically removed. Note that the cmd might be still - * referenced in various entries in the callstack. The - * reference counting on these elements takes care that the - * cmdPtr is deleted on a pop operation (although we do a - * Tcl_DeleteCommandFromToken() below. + * When we try to invoke a deleted object, the cmd (alias) is + * automatically removed. Note that the cmd might be still referenced + * in various entries in the callstack. The reference counting on + * these elements takes care that the cmdPtr is deleted on a pop + * operation (although we do a Tcl_DeleteCommandFromToken() below. */ /*fprintf(stderr, "methodName %s FOUND deleted object with cmd %p my cscPtr %p\n", methodName, cmd, cscPtr);*/ @@ -9184,10 +9182,12 @@ result = NextSearchAndInvoke(interp, MethodName(cscPtr1->objv[0]), cscPtr1->objc, cscPtr1->objv, cscPtr1, 0); } - + /*fprintf(stderr, "==> next %s.%s (obj %s) csc %p returned %d unknown %d\n", - ObjectName(self), methodName, ObjectName(object), cscPtr, result, rst->unknown);*/ - if (rst->unknown) { + ObjectName(self), methodName, ObjectName(object), cscPtr, result, + RUNTIME_STATE(interp)->unknown); */ + + if (RUNTIME_STATE(interp)->unknown) { /* * Unknown handling: We trigger a dispatch to an unknown method. The * appropriate unknown handler is either provided for the current @@ -9564,7 +9564,7 @@ methodName, cl, cl ? ClassName(cl) : "NONE", cmd);*/ } else if (*methodName == ':') { - NsfObject *regObject, *defObject; + NsfObject *regObject; int fromClassNS = 0; /* @@ -9578,7 +9578,7 @@ INCR_REF_COUNT(methodObj); cmd = ResolveMethodName(interp, NULL, methodObj, - NULL, ®Object, &defObject, NULL, &fromClassNS); + NULL, ®Object, NULL, NULL, &fromClassNS); DECR_REF_COUNT(methodObj); if (cmd) { @@ -17666,85 +17666,76 @@ } if (CmdIsNsfObject(cmd)) { - Tcl_Command oldCmd = NULL; - Tcl_Namespace *lookupNsPtr = NULL; - NsfObject *oldTargetObject = NULL, *newTargetObject = (NsfObject *)Tcl_Command_objClientData(cmd); + Tcl_Command oldCmd; + Tcl_Namespace *lookupNsPtr; + NsfObject *oldTargetObject, *newTargetObject; + newTargetObject = (NsfObject *)Tcl_Command_objClientData(cmd); + assert(newTargetObject); + /* * We need to perform a defensive lookup of a previously defined * object-alias under the given methodName. */ - lookupNsPtr = cl ? cl->nsPtr : object->nsPtr; oldCmd = lookupNsPtr ? FindMethod(lookupNsPtr, methodName) : NULL; - if (oldCmd != NULL && CmdIsNsfObject(oldCmd)) { + if (oldCmd != NULL) { oldTargetObject = NsfGetObjectFromCmdPtr(oldCmd); + } else { + oldTargetObject = NULL; } /* - * When registering an object as an alias method, we must handle cases of - * already destroyed alias targets. An object might have been destroyed - * and the alias is left dangling. We do so by bumping the target object's - * ref counter. + * Bump the object reference counter when the new target object is + * different from the old one. Note, that the old target object might be + * NULL, in case the object is used here first. * - * BEWARE: Imagine a simplistic scenario such as: + * The following scenario shows a first registration of a reference + * followed by a redefinition. Only in the first case, the reference + * counter is increased. * * Object create ::foo * Object create ::o { - * :alias FOO ::foo - * :alias FOO ::foo - * :alias FOO ::foo + * :alias FOO ::foo + * :alias FOO ::foo * } * - * Here, without verifying whether one and the same object (::foo) has - * already been registered as an alias method (FOO), each alias - * registration would lead to an increment of ::foo's ref counter. As - * there is no corresponding decrement for each registration, we would end - * up with a skewed ref count. */ - if (oldTargetObject == NULL || oldTargetObject != newTargetObject) { - /*fprintf(stderr,"BUMPING oldTargetObject %p (%s) VS. newTargetObject %p (%s) --- rc %d\n", - oldTargetObject != NULL ? oldTargetObject : NULL, oldTargetObject != NULL ? ObjectName(oldTargetObject) : "n/a", - newTargetObject, ObjectName(newTargetObject), - newTargetObject->refCount);*/ + if (oldTargetObject != newTargetObject) { NsfObjectRefCountIncr(newTargetObject); + } else { + /*fprintf(stderr, "--- don't incr refcount on obj %p %s\n", + newTargetObject, ObjectName(newTargetObject));*/ } /* - * Likewise, there is the risk of finding an aliased object having been "recreated" - * in an unnoticed manner, e.g.: + * The old target object might be already stale (i.e. destroyed, but kept + * alive by the reference counter). In this case, we have to decrement the + * object reference counter and release this object here. * - * nx::Object create ::o - * nx::Object create ::baff - * nx::Object create ::baff::child - * - * ::o alias X ::baff::child - * - * # NsfDeleteChild() cannot (!) not perform an AliasDeleteObjectReference() for the o.X() alias (or the like!) - * nx::Object create ::baff; - * - * nx::Object create ::baff::child; # the child is reborn! - * ::o alias X ::baff::child; # a new alias cmd is created, leaving the old object reference leaking! + * nx::Object create ::x + * nx::Object create ::o {:alias X ::x} * - * In my understanding, we cannot perform an alias-object cleanup in - * NsfDeleteChild(), because we can only do so from the perspective of the - * registration object. A reverse lookup through AliasGet() won't help, - * because a token-based command lookup will give us the *new* cmd for the - * same-named object! Therefore, I suggest the lazy cleanup below. This - * cleanup will only fire if another ::nsf::method::alias statement is - * evaluated on the recreated, yet same-named object. If this is not done, - * the fix above for "taming" the ref counts on identical object targets - * has guaranteed that the aliased, but vanished object will be cleaned up - * properly. + * Destroy the object and create it new + * + * x destroy + * nx::Object create ::x + * + * The recreation of the alias has to decrement the reference counter on + * the old object ::x + * + * o alias X ::x + * + * The test below is exactly the same as for invokes on destroyed aliased + * objects in ObjectDispatchCsc(). */ - if (oldTargetObject != NULL && oldTargetObject->flags & NSF_DELETED) { - /* fprintf(stderr, "--- CLEARING OLD OBJ: %p %s\n", oldTargetObject, ObjectName(oldTargetObject));*/ + /*fprintf(stderr, "--- releasing destroyed object %p refCount %d\n", + oldTargetObject, oldTargetObject->refCount);*/ + assert(oldTargetObject->refCount > 0); AliasDeleteObjectReference(interp, oldCmd); } - /*newObjProc = NsfProcAliasMethod;*/ - } else if (CmdIsProc(cmd)) { /* * When we have a Tcl proc|nsf-method as alias, then use the Index: tests/alias.test =================================================================== diff -u -N -ra24e1f836c3126d0a0e9467bde3a9fa8da901711 -r3cc93845be8bba214f09b9c6ec29e48de99175ff --- tests/alias.test (.../alias.test) (revision a24e1f836c3126d0a0e9467bde3a9fa8da901711) +++ tests/alias.test (.../alias.test) (revision 3cc93845be8bba214f09b9c6ec29e48de99175ff) @@ -666,3 +666,67 @@ ? {c1 baz} 4 ? {c1 vars} {a 2 b 4} } + +# +# Testing aliases to objects and reference counting. +# Check the effects via MEM_COUNT... +# +nx::Test case refcount-object-alias-recreate { + + nx::Object create ::x + nx::Object create ::o { + :alias X ::x + ? {o info method definition X} "::o protected alias X ::x" + :alias X ::x + ? {o info method definition X} "::o protected alias X ::x" + } +} + +nx::Test case refount-destroy-delete1 { + nx::Object create ::x + nx::Object create ::o {:alias X ::x} + + ? {o info method definition X} "::o protected alias X ::x" + + # destroy the object, make sure it does not exist anymore + ? {x destroy} "" + ? {nsf::object::exists x} 0 + + # The alias lookup does still work + ? {o info method definition X} "::o protected alias X ::x" + + # Create the referenced object new + nx::Object create ::x + + # Recreation of the alias, must free refcount to the old object + ? {::o alias X ::x} "::o::X" + + # Recreate the object. On recreation, the object is not freed, + # therefore we test the reference counter is aleady set, and must + # nor be incremented + nx::Object create ::x + ? {::o alias X ::x} "::o::X" +} + +nx::Test case refount-destroy-delete2 { + nx::Object create ::o + nx::Object create ::baff + nx::Object create ::baff::child + + ::o alias X ::baff::child + ? {nsf::object::exists ::baff::child} 1 + ? {o info method definition X} "::o protected alias X ::baff::child" + + nx::Object create ::baff + ? {nsf::object::exists ::baff::child} 0 + + # The alias lookup does still work + ? {o info method definition X} "::o protected alias X ::baff::child" + + # Create the child new + nx::Object create ::baff::child + ? {nsf::object::exists ::baff::child} 1 + + # Recreation of the alias, must free refcount to the old object + ? {::o alias X ::baff::child} "::o::X" +} \ No newline at end of file