In the recent python-dev thread "Bizarre new test failure", we
discovered that subtype_traverse must traverse the type if it is a
heap type, because otherwise some cycles involving a type and its
instance would not be collected. Simplest example:
while 1:
class C(object): pass
C.ref = C()
This program grows without bounds before this fix. (It grows ever
slower since it spends ever more time in the collector.)
Simply adding the right visit() call to subtype_traverse() revealed
other problems. With MvL's help we re-learned that type_clear()
doesn't have to clear *all* references, only the ones that may not be
cleared by other means. Careful analysis (see comments in the code)
revealed that only tp_mro needs to be cleared. (The previous checkin
to this file adds a test for tp_mro==NULL to _PyType_Lookup() that's
essential to prevent crashes due to tp_mro being NULL when
subtype_dealloc() tries to look for a __del__ method.) The same kind
of analysis also revealed that subtype_clear() doesn't need to clear
the instance dict.
With this fix, a useful property of the collector is once again
guaranteed: a single gc.collect() call will clear out all garbage.
(It didn't always before, which put us on the track of this bug.)
Will backport to 2.2.
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 07de6dc..8b51a53 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -290,6 +290,15 @@
}
}
+ if (type->tp_flags & Py_TPFLAGS_HEAPTYPE) {
+ /* For a heaptype, the instances count as references
+ to the type. Traverse the type so the collector
+ can find cycles involving this link. */
+ int err = visit((PyObject *)type, arg);
+ if (err)
+ return err;
+ }
+
if (basetraverse)
return basetraverse(self, visit, arg);
return 0;
@@ -332,12 +341,8 @@
assert(base);
}
- if (type->tp_dictoffset != base->tp_dictoffset) {
- PyObject **dictptr = _PyObject_GetDictPtr(self);
- if (dictptr && *dictptr) {
- PyDict_Clear(*dictptr);
- }
- }
+ /* There's no need to clear the instance dict (if any);
+ the collector will call its tp_clear handler. */
if (baseclear)
return baseclear(self);
@@ -1483,13 +1488,11 @@
static int
type_traverse(PyTypeObject *type, visitproc visit, void *arg)
{
- etype *et;
int err;
- if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE))
- return 0;
-
- et = (etype *)type;
+ /* Because of type_is_gc(), the collector only calls this
+ for heaptypes. */
+ assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE);
#define VISIT(SLOT) \
if (SLOT) { \
@@ -1503,8 +1506,11 @@
VISIT(type->tp_mro);
VISIT(type->tp_bases);
VISIT(type->tp_base);
- VISIT(type->tp_subclasses);
- VISIT(et->slots);
+
+ /* There's no need to visit type->tp_subclasses or
+ ((etype *)type)->slots, because they can't be involved
+ in cycles; tp_subclasses is a list of weak references,
+ and slots is a tuple of strings. */
#undef VISIT
@@ -1514,13 +1520,11 @@
static int
type_clear(PyTypeObject *type)
{
- etype *et;
PyObject *tmp;
- if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE))
- return 0;
-
- et = (etype *)type;
+ /* Because of type_is_gc(), the collector only calls this
+ for heaptypes. */
+ assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE);
#define CLEAR(SLOT) \
if (SLOT) { \
@@ -1529,18 +1533,32 @@
Py_DECREF(tmp); \
}
- CLEAR(type->tp_dict);
- CLEAR(type->tp_cache);
- CLEAR(type->tp_mro);
- CLEAR(type->tp_bases);
- CLEAR(type->tp_base);
- CLEAR(type->tp_subclasses);
- CLEAR(et->slots);
+ /* The only field we need to clear is tp_mro, which is part of a
+ hard cycle (its first element is the class itself) that won't
+ be broken otherwise (it's a tuple and tuples don't have a
+ tp_clear handler). None of the other fields need to be
+ cleared, and here's why:
- if (type->tp_doc != NULL) {
- PyObject_FREE(type->tp_doc);
- type->tp_doc = NULL;
- }
+ tp_dict:
+ It is a dict, so the collector will call its tp_clear.
+
+ tp_cache:
+ Not used; if it were, it would be a dict.
+
+ tp_bases, tp_base:
+ If these are involved in a cycle, there must be at least
+ one other, mutable object in the cycle, e.g. a base
+ class's dict; the cycle will be broken that way.
+
+ tp_subclasses:
+ A list of weak references can't be part of a cycle; and
+ lists have their own tp_clear.
+
+ slots (in etype):
+ A tuple of strings can't be part of a cycle.
+ */
+
+ CLEAR(type->tp_mro);
#undef CLEAR
@@ -2157,10 +2175,8 @@
PyTypeObject *base;
int i, n;
- if (type->tp_flags & Py_TPFLAGS_READY) {
- assert(type->tp_dict != NULL);
+ if (type->tp_flags & Py_TPFLAGS_READY)
return 0;
- }
assert((type->tp_flags & Py_TPFLAGS_READYING) == 0);
type->tp_flags |= Py_TPFLAGS_READYING;