Index: TODO =================================================================== diff -u -r5371aaaf333c92616a8baf6e521df386909b876d -rfbd7f24c6111a5328c15bb7c47e9a45c9928357f --- TODO (.../TODO) (revision 5371aaaf333c92616a8baf6e521df386909b876d) +++ TODO (.../TODO) (revision fbd7f24c6111a5328c15bb7c47e9a45c9928357f) @@ -3796,6 +3796,12 @@ when called without a callstack - added regression test +nsf.c: +- change argument of IsMetaClass() to type *NsfObject +- provide basic pretection against most likely unintended + deletion/overwriting of base classes. +- extend regression test + ======================================================================== TODO: Index: generic/nsf.c =================================================================== diff -u -r5371aaaf333c92616a8baf6e521df386909b876d -rfbd7f24c6111a5328c15bb7c47e9a45c9928357f --- generic/nsf.c (.../nsf.c) (revision 5371aaaf333c92616a8baf6e521df386909b876d) +++ generic/nsf.c (.../nsf.c) (revision fbd7f24c6111a5328c15bb7c47e9a45c9928357f) @@ -284,7 +284,7 @@ static void GuardDel(NsfCmdList *filterCL); /* properties of objects and classes */ -static int IsBaseClass(NsfClass *cl); +static int IsBaseClass(NsfObject *cl); static int IsMetaClass(Tcl_Interp *interp, NsfClass *cl, int withMixins); static int IsSubType(NsfClass *subcl, NsfClass *cl); static NsfClass *DefaultSuperClass(Tcl_Interp *interp, NsfClass *cl, NsfClass *mcl, int isMeta); @@ -1471,7 +1471,7 @@ NsfClass *cl; Tcl_DString ds, *dsPtr = &ds; - if (unlikely(pPtr->flags & NSF_ARG_BASECLASS) && !IsBaseClass((NsfClass *)object)) { + if (unlikely(pPtr->flags & NSF_ARG_BASECLASS) && !IsBaseClass(object)) { what = "baseclass"; goto type_error; } @@ -1995,13 +1995,23 @@ Tcl_HashEntry *hPtr; assert(cl); - hPtr = Tcl_CreateHashEntry(&cl->instances, (char *)object, NULL); - - /*if (hPtr == NULL) { - fprintf(stderr, "instance %s is not an instance of %s\n", ObjectName(object), ClassName(cl)); - }*/ - assert(hPtr); - Tcl_DeleteHashEntry(hPtr); + /* + * If we are during a delete, which should not happen under normal + * operations, prevent an abort due to a deleted hash table. + */ + if (cl->object.flags & NSF_DURING_DELETE) { + NsfLog(cl->object.teardown, NSF_LOG_WARN, + "Class which should loose instance is currently being deleted: %s", + ClassName(cl)); + } else { + hPtr = Tcl_CreateHashEntry(&cl->instances, (char *)object, NULL); + + /*if (hPtr == NULL) { + fprintf(stderr, "instance %s is not an instance of %s\n", ObjectName(object), ClassName(cl)); + }*/ + assert(hPtr); + Tcl_DeleteHashEntry(hPtr); + } } /* @@ -4162,10 +4172,9 @@ * the command anyway, since its parent is currently being * deleted. */ - NsfLog(interp, NSF_LOG_NOTICE, "Destroy failed for object %s, perform low level deletion", - ObjectName(object)); - if (object->teardown) { + NsfLog(interp, NSF_LOG_NOTICE, "Destroy failed for object %s, perform low level deletion", + ObjectName(object)); CallStackDestroyObject(interp, object); } } @@ -10206,7 +10215,7 @@ * Skip entries until the first base class. */ for (; classList; classList = classList->nextPtr) { - if (IsBaseClass(classList->cl)) {break;} + if (IsBaseClass(&classList->cl->object)) {break;} } cl = SearchPLMethod(classList, methodName, &cmd, NSF_CMD_CALL_PRIVATE_METHOD); } else { @@ -10407,8 +10416,9 @@ */ if (rst->exitHandlerDestroyRound == NSF_EXITHANDLER_ON_PHYSICAL_DESTROY || (object->flags & NSF_DESTROY_CALLED) - ) + ) { return TCL_OK; + } /*fprintf(stderr, " DispatchDestroyMethod obj %p flags %.6x active %d\n", object, object->flags, object->activationCount); */ @@ -14657,8 +14667,8 @@ } static int -IsBaseClass(NsfClass *cl) { - return cl->object.flags & (NSF_IS_ROOT_META_CLASS|NSF_IS_ROOT_CLASS); +IsBaseClass(NsfObject *object) { + return object->flags & (NSF_IS_ROOT_META_CLASS|NSF_IS_ROOT_CLASS); } @@ -17301,10 +17311,10 @@ * If the method is object specific, it can't be from a baseclass and must * be application specific. */ - return (withSource == SourceApplicationIdx && !IsBaseClass((NsfClass *)object)); + return (withSource == SourceApplicationIdx && !IsBaseClass(object)); } - isBaseClass = IsBaseClass(cl); + isBaseClass = IsBaseClass(&cl->object); if (withSource == SourceBaseclassesIdx && isBaseClass) { return 1; } else if (withSource == SourceApplicationIdx && !isBaseClass) { @@ -21093,6 +21103,15 @@ NsfODestroyMethod(Tcl_Interp *interp, NsfObject *object) { PRINTOBJ("NsfODestroyMethod", object); + /* + * Provide protection against destroy on base classes. + */ + if (unlikely(IsBaseClass(object))) { + if (RUNTIME_STATE(interp)->exitHandlerDestroyRound != NSF_EXITHANDLER_ON_SOFT_DESTROY) { + return NsfPrintError(interp, "Cannot destroy base class %s", ObjectName(object)); + } + } + /*fprintf(stderr,"NsfODestroyMethod %p %s flags %.6x activation %d cmd %p cmd->flags %.6x\n", object, ((Command *)object->id)->flags == 0 ? ObjectName(object) : "(deleted)", object->flags, object->activationCount, object->id, ((Command *)object->id)->flags);*/ @@ -21670,7 +21689,16 @@ newObject ? IsMetaClass(interp, newObject->cl, 1) : 0 );*/ + /* + * Provide protection against recreation if base classes. + */ + if (newObject && unlikely(IsBaseClass(newObject))) { + result = NsfPrintError(interp, "Cannot recreate base class %s", ObjectName(newObject)); + goto create_method_exit; + } + + /* * Don't allow to * - recreate an object as a class, * - recreate a class as an object, and to @@ -21712,7 +21740,7 @@ */ /*fprintf(stderr, "alloc ... %s newObject %p \n", ObjStr(nameObj), newObject);*/ - + if (CallDirectly(interp, &cl->object, NSF_c_alloc_idx, &methodObj)) { result = NsfCAllocMethod_(interp, cl, nameObj, parentNsPtr); } else { @@ -22165,13 +22193,11 @@ break; case ObjectkindMetaclassIdx: - success = NsfObjectIsClass(object) - && IsMetaClass(interp, (NsfClass *)object, 1); + success = NsfObjectIsClass(object) && IsMetaClass(interp, (NsfClass *)object, 1); break; case ObjectkindBaseclassIdx: - success = NsfObjectIsClass(object) - && IsBaseClass((NsfClass *)object); + success = NsfObjectIsClass(object) && IsBaseClass(object); break; } Tcl_SetIntObj(Tcl_GetObjResult(interp), success); @@ -23347,7 +23373,7 @@ NsfCmdList *entry, *lastEntry; int deleted = 0; - /* fprintf(stderr, "FreeAllNsfObjectsAndClasses in %p\n", interp); */ + /*fprintf(stderr, "FreeAllNsfObjectsAndClasses in %p\n", interp);*/ RUNTIME_STATE(interp)->exitHandlerDestroyRound = NSF_EXITHANDLER_ON_PHYSICAL_DESTROY; @@ -23477,7 +23503,7 @@ && !ObjectHasChildren((NsfObject *)cl) && !ClassHasInstances(cl) && !ClassHasSubclasses(cl) - && !IsBaseClass(cl) + && !IsBaseClass(&cl->object) ) { /*fprintf(stderr, " ... delete class %s %p\n", ClassName(cl), cl); */ assert(cl->object.id); Index: tests/object-system.test =================================================================== diff -u -r2152f4606b7c4e81fd18018c7c43bf29961a9d1b -rfbd7f24c6111a5328c15bb7c47e9a45c9928357f --- tests/object-system.test (.../object-system.test) (revision 2152f4606b7c4e81fd18018c7c43bf29961a9d1b) +++ tests/object-system.test (.../object-system.test) (revision fbd7f24c6111a5328c15bb7c47e9a45c9928357f) @@ -317,16 +317,36 @@ # -# Test instances of diamond class structure. Leave class structure -# around until exit to test handling of pot. duplicated entries +# Test protection against (most likely unintended) deletion of base +# classes. # +? {catch {nx::Object destroy}} 1 +? {::nsf::object::exists nx::Object} 1 +? {catch {nx::Object create nx::Object}} 1 +? {::nsf::object::exists nx::Object} 1 +? {catch {nx::Object create nx::Class}} 1 +? {::nsf::object::exists nx::Class} 1 +? {catch {nx::Class create nx::Object}} 1 +? {catch {nx::Class create nx::Class}} 1 +? {catch {rename nx::Object ""}} 1 +? {::nsf::object::exists nx::Object} 1 +? {catch {rename nx::Object ""}} 1 +? {::nsf::object::exists nx::Object} 1 +? {catch {rename nx::Class ""}} 1 +? {::nsf::object::exists nx::Class} 1 + +# +# Test instances of diamond class structure. +# +# Leave class structure around until exit to test handling of +# potentially duplicated entries during final cleanup +# nx::Class create A nx::Class create B1 -superclass A nx::Class create B2 -superclass A nx::Class create C -superclass {B1 B2} ? {C create c1} ::c1 ? {A info instances -closure} ::c1 - puts stderr ===EXIT ::nsf::configure dtrace off