From c032f5043e88e8f54ac32f526413a3b62c9a20f4 Mon Sep 17 00:00:00 2001 From: pooryorick Date: Wed, 29 Nov 2017 12:01:21 +0000 Subject: Fix for [6bca38d59b], TclOO segmentation fault cleaning up objects that that have mixed themselves into themselves. --- generic/tclOO.c | 128 ++++++++++++++++++++++++++++++++------------------------ tests/oo.test | 33 +++++++++++++-- 2 files changed, 103 insertions(+), 58 deletions(-) diff --git a/generic/tclOO.c b/generic/tclOO.c index 93abf3f..5404abc 100644 --- a/generic/tclOO.c +++ b/generic/tclOO.c @@ -913,28 +913,30 @@ ReleaseClassContents( } if (!IsRootClass(oPtr)) { FOREACH(instancePtr, clsPtr->instances) { - int j; - if (instancePtr->selfCls == clsPtr) { - instancePtr->flags |= CLASS_GONE; - } - for(j=0 ; jmixins.num ; j++) { - Class *mixin = instancePtr->mixins.list[j]; - Class *nextMixin = NULL; - if (mixin == clsPtr) { - if (j < instancePtr->mixins.num - 1) { - nextMixin = instancePtr->mixins.list[j+1]; - } - if (j == 0) { - instancePtr->mixins.num = 0; - instancePtr->mixins.list = NULL; - } else { - instancePtr->mixins.list[j-1] = nextMixin; + if (instancePtr != oPtr) { + int j; + if (instancePtr->selfCls == clsPtr) { + instancePtr->flags |= CLASS_GONE; + } + for(j=0 ; jmixins.num ; j++) { + Class *mixin = instancePtr->mixins.list[j]; + Class *nextMixin = NULL; + if (mixin == clsPtr) { + if (j < instancePtr->mixins.num - 1) { + nextMixin = instancePtr->mixins.list[j+1]; + } + if (j == 0) { + instancePtr->mixins.num = 0; + instancePtr->mixins.list = NULL; + } else { + instancePtr->mixins.list[j-1] = nextMixin; + } + instancePtr->mixins.num -= 1; } - instancePtr->mixins.num -= 1; } - } - if (instancePtr != NULL && !IsRoot(instancePtr)) { - AddRef(instancePtr); + if (instancePtr != NULL && !IsRoot(instancePtr)) { + AddRef(instancePtr); + } } } } @@ -944,13 +946,15 @@ ReleaseClassContents( */ FOREACH(mixinSubclassPtr, clsPtr->mixinSubs) { - if (!Deleted(mixinSubclassPtr->thisPtr)) { - Tcl_DeleteCommandFromToken(interp, - mixinSubclassPtr->thisPtr->command); + if (mixinSubclassPtr != clsPtr) { + if (!Deleted(mixinSubclassPtr->thisPtr)) { + Tcl_DeleteCommandFromToken(interp, + mixinSubclassPtr->thisPtr->command); + } + ClearMixins(mixinSubclassPtr); + DelRef(mixinSubclassPtr->thisPtr); + DelRef(mixinSubclassPtr); } - ClearMixins(mixinSubclassPtr); - DelRef(mixinSubclassPtr->thisPtr); - DelRef(mixinSubclassPtr); } if (clsPtr->mixinSubs.list != NULL) { ckfree(clsPtr->mixinSubs.list); @@ -985,19 +989,21 @@ ReleaseClassContents( if (!IsRootClass(oPtr)) { FOREACH(instancePtr, clsPtr->instances) { - if (instancePtr == NULL || IsRoot(instancePtr)) { - continue; - } - if (!Deleted(instancePtr)) { - Tcl_DeleteCommandFromToken(interp, instancePtr->command); - /* - * Tcl_DeleteCommandFromToken() may have done to whole - * job for us. Roll back and check again. - */ - i--; - continue; + if (instancePtr != oPtr) { + if (instancePtr == NULL || IsRoot(instancePtr)) { + continue; + } + if (!Deleted(instancePtr)) { + Tcl_DeleteCommandFromToken(interp, instancePtr->command); + /* + * Tcl_DeleteCommandFromToken() may have done to whole + * job for us. Roll back and check again. + */ + i--; + continue; + } + DelRef(instancePtr); } - DelRef(instancePtr); } } if (clsPtr->instances.list != NULL) { @@ -1084,6 +1090,10 @@ ReleaseClassContents( ckfree(clsPtr->variables.list); } + /* Tell oPtr that it's class is gone so that it doesn't try to remove + * itself from it's classe's list of instances + */ + oPtr->flags |= CLASS_GONE; DelRef(clsPtr); } @@ -1177,22 +1187,6 @@ ObjectNamespaceDeleted( } /* - * The class of objects needs some special care; if it is deleted (and - * we're not killing the whole interpreter) we force the delete of the - * class of classes now as well. Due to the incestuous nature of those two - * classes, if one goes the other must too and yet the tangle can - * sometimes not go away automatically; we force it here. [Bug 2962664] - */ - if (!Tcl_InterpDeleted(interp) && IsRootObject(oPtr) - && !Deleted(fPtr->classCls->thisPtr)) { - Tcl_DeleteCommandFromToken(interp, fPtr->classCls->thisPtr->command); - } - - if (oPtr->classPtr != NULL) { - ReleaseClassContents(interp, oPtr); - } - - /* * Splice the object out of its context. After this, we must *not* call * methods on the object. */ @@ -1202,7 +1196,7 @@ ObjectNamespaceDeleted( } FOREACH(mixinPtr, oPtr->mixins) { - if (mixinPtr) { + if (mixinPtr && mixinPtr != oPtr->classPtr) { TclOORemoveFromInstances(oPtr, mixinPtr); } } @@ -1251,6 +1245,30 @@ ObjectNamespaceDeleted( } /* + * Because an object can be a class that is an instance of itself, the + * A class object's class structure should only be cleaned after most of + * the cleanup on the object is done. + */ + + + /* + * The class of objects needs some special care; if it is deleted (and + * we're not killing the whole interpreter) we force the delete of the + * class of classes now as well. Due to the incestuous nature of those two + * classes, if one goes the other must too and yet the tangle can + * sometimes not go away automatically; we force it here. [Bug 2962664] + */ + if (!Tcl_InterpDeleted(interp) && IsRootObject(oPtr) + && !Deleted(fPtr->classCls->thisPtr)) { + Tcl_DeleteCommandFromToken(interp, fPtr->classCls->thisPtr->command); + } + + if (oPtr->classPtr != NULL) { + ReleaseClassContents(interp, oPtr); + } + + + /* * Delete the object structure itself. */ diff --git a/tests/oo.test b/tests/oo.test index c44ec18..556d529 100644 --- a/tests/oo.test +++ b/tests/oo.test @@ -1501,7 +1501,7 @@ test oo-11.5 {OO: cleanup} { test oo-11.6 { OO: cleanup ReleaseClassContents() where class is mixed into one of its instances -} { +} -body { oo::class create obj1 ::oo::define obj1 {self mixin [self]} @@ -1509,12 +1509,14 @@ test oo-11.6 { ::oo::objdefine obj2 {mixin [self]} ::oo::copy obj2 obj3 - trace add command obj3 delete [list obj3 dying] + rename obj3 {} rename obj2 {} # No segmentation fault return done -} done +} -cleanup { + rename obj1 {} +} -result done test oo-12.1 {OO: filters} { oo::class create Aclass @@ -3867,6 +3869,31 @@ test oo-35.5 {Bug 1a56550e96: introspectors must traverse mixin links correctly} } -cleanup { base destroy } -result {{c d e} {c d e}} +test oo-35.6 { + Bug : teardown of an object that is a class that is an instance of itself +} -setup { + oo::class create obj + + oo::copy obj obj1 obj1 + oo::objdefine obj1 { + mixin obj1 obj + } + oo::copy obj1 obj2 + oo::objdefine obj2 { + mixin obj2 obj1 + } +} -body { + rename obj2 {} + rename obj1 {} + # doesn't crash + return done +} -cleanup { + rename obj {} +} -result done + + + + test oo-36.1 {TIP #470: introspection within oo::define} { oo::define oo::object self } ::oo::object -- cgit v0.12