Backport two patches that belong together:

(2.150)
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.)

(2.151)
Undo the last chunk of the previous patch, putting back a useful
assert into PyType_Ready(): now that we're not clearing tp_dict, we
can assert that it's non-NULL again.

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 29a889cf6ae9e596f611e51c41c4dffd4c081edc..c7a42548f90a0c6b372fdc6e32969760789820f6 100644 (file)
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -287,6 +287,15 @@ subtype_traverse(PyObject *self, visitproc visit, void *arg)
                }
        }
 
+       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;
@@ -329,12 +338,8 @@ subtype_clear(PyObject *self)
                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);
@@ -1438,13 +1443,11 @@ static char type_doc[] =
 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) { \
@@ -1458,8 +1461,11 @@ type_traverse(PyTypeObject *type, visitproc visit, void *arg)
        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
 
@@ -1469,13 +1475,11 @@ type_traverse(PyTypeObject *type, visitproc visit, void *arg)
 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) { \
@@ -1484,18 +1488,32 @@ type_clear(PyTypeObject *type)
                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