Tcl Source Code

Check-in [313a7c1b8a]
Login

Many hyperlinks are disabled.
Use anonymous login to enable hyperlinks.

Overview
Comment:Fix for [6bca38d59b], TclOO segmentation fault cleaning up objects that that have mixed themselves into themselves.
Downloads: Tarball | ZIP archive | SQL archive
Timelines: family | ancestors | descendants | both | core-8-branch
Files: files | file ages | folders
SHA3-256: 313a7c1b8a91b234e79bbe9aaaed2472601ad3bd075b9fb0d1b5313fe282ab97
User & Date: pooryorick 2017-11-29 12:01:21
References
2017-11-29
12:06 Closed ticket [6bca38d59b]: TclOO segmentation fault cleaning up objects that that have mixed themselves into themselves. plus 6 other changes artifact: 808b062ef0 user: pooryorick
Context
2018-02-15
09:10
Fix for [6bca38d59b], TclOO segmentation fault cleaning up objects that that have mixed themselves i... check-in: 3908414450 user: pooryorick tags: pyk-backport-to-8-6
2017-11-29
12:28
merge core-8-6-branch check-in: e45dcdac38 user: jan.nijtmans tags: core-8-branch
12:01
Fix for [6bca38d59b], TclOO segmentation fault cleaning up objects that that have mixed themselves i... check-in: 313a7c1b8a user: pooryorick tags: core-8-branch
11:05
merge core-8-6-branch check-in: 8976a447aa user: jan.nijtmans tags: core-8-branch
Changes
Hide Diffs Unified Diffs Ignore Whitespace Patch

Changes to generic/tclOO.c.

909
910
911
912
913
914
915

916
917
918
919
920
921
922
923
924
925
926
927
928
929
930
931
932
933
934
935
936
937

938
939
940
941
942
943
944
945
946

947
948
949
950
951
952
953

954
955
956
957
958
959
960
	if (subclassPtr != NULL && !IsRoot(subclassPtr)) {
	    AddRef(subclassPtr);
	    AddRef(subclassPtr->thisPtr);
	}
    }
    if (!IsRootClass(oPtr)) {
	FOREACH(instancePtr, clsPtr->instances) {

	    int j;
	    if (instancePtr->selfCls == clsPtr) {
		instancePtr->flags |= CLASS_GONE;
	    }
	    for(j=0 ; j<instancePtr->mixins.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;
		}
	    }
	    if (instancePtr != NULL && !IsRoot(instancePtr)) {
		AddRef(instancePtr);

	    }
	}
    }

    /*
     * Squelch classes that this class has been mixed into.
     */

    FOREACH(mixinSubclassPtr, clsPtr->mixinSubs) {

	if (!Deleted(mixinSubclassPtr->thisPtr)) {
	    Tcl_DeleteCommandFromToken(interp,
		    mixinSubclassPtr->thisPtr->command);
	}
	ClearMixins(mixinSubclassPtr);
	DelRef(mixinSubclassPtr->thisPtr);
	DelRef(mixinSubclassPtr);

    }
    if (clsPtr->mixinSubs.list != NULL) {
	ckfree(clsPtr->mixinSubs.list);
	clsPtr->mixinSubs.list = NULL;
	clsPtr->mixinSubs.num = 0;
    }








>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
>









>
|
|
|
|
|
|
|
>







909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
926
927
928
929
930
931
932
933
934
935
936
937
938
939
940
941
942
943
944
945
946
947
948
949
950
951
952
953
954
955
956
957
958
959
960
961
962
963
964
	if (subclassPtr != NULL && !IsRoot(subclassPtr)) {
	    AddRef(subclassPtr);
	    AddRef(subclassPtr->thisPtr);
	}
    }
    if (!IsRootClass(oPtr)) {
	FOREACH(instancePtr, clsPtr->instances) {
	    if (instancePtr != oPtr) {
		int j;
		if (instancePtr->selfCls == clsPtr) {
		    instancePtr->flags |= CLASS_GONE;
		}
		for(j=0 ; j<instancePtr->mixins.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;
		    }
		}
		if (instancePtr != NULL && !IsRoot(instancePtr)) {
		    AddRef(instancePtr);
		}
	    }
	}
    }

    /*
     * Squelch classes that this class has been mixed into.
     */

    FOREACH(mixinSubclassPtr, clsPtr->mixinSubs) {
	if (mixinSubclassPtr != clsPtr) {
	    if (!Deleted(mixinSubclassPtr->thisPtr)) {
		Tcl_DeleteCommandFromToken(interp,
			mixinSubclassPtr->thisPtr->command);
	    }
	    ClearMixins(mixinSubclassPtr);
	    DelRef(mixinSubclassPtr->thisPtr);
	    DelRef(mixinSubclassPtr);
	}
    }
    if (clsPtr->mixinSubs.list != NULL) {
	ckfree(clsPtr->mixinSubs.list);
	clsPtr->mixinSubs.list = NULL;
	clsPtr->mixinSubs.num = 0;
    }

981
982
983
984
985
986
987

988
989
990
991
992
993
994
995
996
997
998
999
1000

1001
1002
1003
1004
1005
1006
1007

    /*
     * Squelch instances of this class (includes objects we're mixed into).
     */

    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;
	    }
	    DelRef(instancePtr);

	}
    }
    if (clsPtr->instances.list != NULL) {
	ckfree(clsPtr->instances.list);
	clsPtr->instances.list = NULL;
	clsPtr->instances.num = 0;
    }







>
|
|
|
|
|
|
|
|
|
|
|
|
|
>







985
986
987
988
989
990
991
992
993
994
995
996
997
998
999
1000
1001
1002
1003
1004
1005
1006
1007
1008
1009
1010
1011
1012
1013

    /*
     * Squelch instances of this class (includes objects we're mixed into).
     */

    if (!IsRootClass(oPtr)) {
	FOREACH(instancePtr, clsPtr->instances) {
	    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);
	    }
	}
    }
    if (clsPtr->instances.list != NULL) {
	ckfree(clsPtr->instances.list);
	clsPtr->instances.list = NULL;
	clsPtr->instances.num = 0;
    }
1080
1081
1082
1083
1084
1085
1086




1087
1088
1089
1090
1091
1092
1093
    FOREACH(variableObj, clsPtr->variables) {
	TclDecrRefCount(variableObj);
    }
    if (i) {
	ckfree(clsPtr->variables.list);
    }





    DelRef(clsPtr);

}

/*
 * ----------------------------------------------------------------------
 *







>
>
>
>







1086
1087
1088
1089
1090
1091
1092
1093
1094
1095
1096
1097
1098
1099
1100
1101
1102
1103
    FOREACH(variableObj, clsPtr->variables) {
	TclDecrRefCount(variableObj);
    }
    if (i) {
	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);

}

/*
 * ----------------------------------------------------------------------
 *
1172
1173
1174
1175
1176
1177
1178
1179
1180
1181
1182
1183
1184
1185
1186
1187
1188
1189
1190
1191
1192
1193
1194
1195
1196
1197
1198
1199
1200
1201
1202
1203
1204
1205
1206
1207
1208
1209
1210
1211
1212
    }
    oPtr->command = NULL;

    if (oPtr->myCommand) {
	Tcl_DeleteCommandFromToken(oPtr->fPtr->interp, oPtr->myCommand);
    }

    /*
     * 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.
     */

    if (!IsRootObject(oPtr) && !(oPtr->flags & CLASS_GONE)) {
	TclOORemoveFromInstances(oPtr, oPtr->selfCls);
    }

    FOREACH(mixinPtr, oPtr->mixins) {
	if (mixinPtr) {
	    TclOORemoveFromInstances(oPtr, mixinPtr);
	}
    }
    if (i) {
	ckfree(oPtr->mixins.list);
    }








<
<
<
<
<
<
<
<
<
<
<
<
<
<
<
<










|







1182
1183
1184
1185
1186
1187
1188
















1189
1190
1191
1192
1193
1194
1195
1196
1197
1198
1199
1200
1201
1202
1203
1204
1205
1206
    }
    oPtr->command = NULL;

    if (oPtr->myCommand) {
	Tcl_DeleteCommandFromToken(oPtr->fPtr->interp, oPtr->myCommand);
    }

















    /*
     * Splice the object out of its context. After this, we must *not* call
     * methods on the object.
     */

    if (!IsRootObject(oPtr) && !(oPtr->flags & CLASS_GONE)) {
	TclOORemoveFromInstances(oPtr, oPtr->selfCls);
    }

    FOREACH(mixinPtr, oPtr->mixins) {
	if (mixinPtr && mixinPtr != oPtr->classPtr) {
	    TclOORemoveFromInstances(oPtr, mixinPtr);
	}
    }
    if (i) {
	ckfree(oPtr->mixins.list);
    }

1245
1246
1247
1248
1249
1250
1251
























1252
1253
1254
1255
1256
1257
1258
	FOREACH_HASH(metadataTypePtr, value, oPtr->metadataPtr) {
	    metadataTypePtr->deleteProc(value);
	}
	Tcl_DeleteHashTable(oPtr->metadataPtr);
	ckfree(oPtr->metadataPtr);
	oPtr->metadataPtr = NULL;
    }

























    /*
     * Delete the object structure itself.
     */

    oPtr->classPtr = NULL;
    oPtr->namespacePtr = NULL;







>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>







1239
1240
1241
1242
1243
1244
1245
1246
1247
1248
1249
1250
1251
1252
1253
1254
1255
1256
1257
1258
1259
1260
1261
1262
1263
1264
1265
1266
1267
1268
1269
1270
1271
1272
1273
1274
1275
1276
	FOREACH_HASH(metadataTypePtr, value, oPtr->metadataPtr) {
	    metadataTypePtr->deleteProc(value);
	}
	Tcl_DeleteHashTable(oPtr->metadataPtr);
	ckfree(oPtr->metadataPtr);
	oPtr->metadataPtr = NULL;
    }

    /*
     *  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.
     */

    oPtr->classPtr = NULL;
    oPtr->namespacePtr = NULL;

Changes to tests/oo.test.

1497
1498
1499
1500
1501
1502
1503
1504
1505
1506
1507
1508
1509
1510
1511
1512
1513
1514
1515
1516


1517
1518
1519
1520
1521
1522
1523
1524
    # No segmentation fault
    return done
} done

test oo-11.6 {
    OO: cleanup ReleaseClassContents() where class is mixed into one of its
    instances
} {
    oo::class create obj1
    ::oo::define obj1 {self mixin [self]}

    ::oo::copy obj1 obj2
    ::oo::objdefine obj2 {mixin [self]}

    ::oo::copy obj2 obj3
    trace add command obj3 delete [list obj3 dying]
    rename obj2 {}

    # No segmentation fault
    return done


} done

test oo-12.1 {OO: filters} {
    oo::class create Aclass
    Aclass create Aobject
    oo::define Aclass {
	method concatenate args {
	    global result







|







|




>
>
|







1497
1498
1499
1500
1501
1502
1503
1504
1505
1506
1507
1508
1509
1510
1511
1512
1513
1514
1515
1516
1517
1518
1519
1520
1521
1522
1523
1524
1525
1526
    # No segmentation fault
    return done
} done

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]}

    ::oo::copy obj1 obj2
    ::oo::objdefine obj2 {mixin [self]}

    ::oo::copy obj2 obj3
    rename obj3 {}
    rename obj2 {}

    # No segmentation fault
    return done
} -cleanup {
    rename obj1 {}
} -result done

test oo-12.1 {OO: filters} {
    oo::class create Aclass
    Aclass create Aobject
    oo::define Aclass {
	method concatenate args {
	    global result
3863
3864
3865
3866
3867
3868
3869

























3870
3871
3872
3873
3874
3875
3876
	method e {} {}
    }
    E create e1
    list [lsort [info class methods E -all]] [lsort [info object methods e1 -all]]
} -cleanup {
    base destroy
} -result {{c d e} {c d e}}

























test oo-36.1 {TIP #470: introspection within oo::define} {
    oo::define oo::object self
} ::oo::object
test oo-36.2 {TIP #470: introspection within oo::define} -setup {
    oo::class create Cls
} -body {
    oo::define Cls self







>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>







3865
3866
3867
3868
3869
3870
3871
3872
3873
3874
3875
3876
3877
3878
3879
3880
3881
3882
3883
3884
3885
3886
3887
3888
3889
3890
3891
3892
3893
3894
3895
3896
3897
3898
3899
3900
3901
3902
3903
	method e {} {}
    }
    E create e1
    list [lsort [info class methods E -all]] [lsort [info object methods e1 -all]]
} -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
test oo-36.2 {TIP #470: introspection within oo::define} -setup {
    oo::class create Cls
} -body {
    oo::define Cls self