[Python-Dev] Bizarre new test failure

[Guido van Rossum]

> > When tp_dict is cleared, this can remove the __del__ method before it
> > can be called (it is called by the instance's tp_dealloc).  
> 
> That cannot happen: an object whose type has an __del__ cannot refer
> to an object for which tp_clear has been called. Objects with
> finalizers go into gc.garbage, so in this case, the type is
> resurrected, and not cleared.

You're right!

> > tp_mro participates in a cycle too: it is a tuple whose first element
> > is the type itself.  Tuples are immutable, so the tp_clear for tuples
> > doesn't do anything.  So type_clear is our only hope to break this
> > cycle.
> 
> I see. So tp_mro must be cleared in tp_clear; it's not used from
> subtype_dealloc, so it won't cause problems to clear it.

You've convinced me.  Here's a patch that only touches typeobject.c.
It doesn't add any fields, and it doesn't require multiple collections
to clear out cycles involving a class and its type.  I like it!

(Note: at the top of type_traverse() and type_clear(), there used to
be code saying "if not a heaptype, return".  That code was never
necessary, because the collector doesn't call the traverse or clear
hooks when tp_is_gc() returns false -- which it does when the heaptype
flag isn't set.  So I replaced these two with an assert that this is a
heaptype.)

Index: typeobject.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Objects/typeobject.c,v
retrieving revision 2.148
diff -c -c -r2.148 typeobject.c
*** typeobject.c	4 Jun 2002 19:52:53 -0000	2.148
--- typeobject.c	9 Jun 2002 23:05:47 -0000
***************
*** 290,295 ****
--- 290,301 ----
  		}
  	}
  
+ 	if (type->tp_flags & Py_TPFLAGS_HEAPTYPE) {
+ 		int err = visit((PyObject *)type, arg);
+ 		if (err)
+ 			return err;
+ 	}
+ 
  	if (basetraverse)
  		return basetraverse(self, visit, arg);
  	return 0;
***************
*** 1323,1329 ****
  			return NULL;
  		}
  		mro = type->tp_mro;
! 		assert(mro != NULL);
  	}
  	assert(PyTuple_Check(mro));
  	n = PyTuple_GET_SIZE(mro);
--- 1329,1336 ----
  			return NULL;
  		}
  		mro = type->tp_mro;
! 		if (mro == NULL)
! 			return NULL;
  	}
  	assert(PyTuple_Check(mro));
  	n = PyTuple_GET_SIZE(mro);
***************
*** 1335,1341 ****
  			assert(PyType_Check(base));
  			dict = ((PyTypeObject *)base)->tp_dict;
  		}
! 		assert(dict && PyDict_Check(dict));
  		res = PyDict_GetItem(dict, name);
  		if (res != NULL)
  			return res;
--- 1342,1349 ----
  			assert(PyType_Check(base));
  			dict = ((PyTypeObject *)base)->tp_dict;
  		}
! 		if (dict == NULL || !PyDict_Check(dict))
! 			continue;
  		res = PyDict_GetItem(dict, name);
  		if (res != NULL)
  			return res;
***************
*** 1495,1502 ****
  	etype *et;
  	int err;
  
! 	if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE))
! 		return 0;
  
  	et = (etype *)type;
  
--- 1503,1509 ----
  	etype *et;
  	int err;
  
! 	assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE);
  
  	et = (etype *)type;
  
***************
*** 1512,1519 ****
  	VISIT(type->tp_mro);
  	VISIT(type->tp_bases);
  	VISIT(type->tp_base);
- 	VISIT(type->tp_subclasses);
- 	VISIT(et->slots);
  
  #undef VISIT
  
--- 1519,1524 ----
***************
*** 1526,1533 ****
  	etype *et;
  	PyObject *tmp;
  
! 	if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE))
! 		return 0;
  
  	et = (etype *)type;
  
--- 1531,1537 ----
  	etype *et;
  	PyObject *tmp;
  
! 	assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE);
  
  	et = (etype *)type;
  
***************
*** 1541,1555 ****
  	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);
- 
- 	if (type->tp_doc != NULL) {
- 		PyObject_FREE(type->tp_doc);
- 		type->tp_doc = NULL;
- 	}
  
  #undef CLEAR
  
--- 1545,1550 ----
***************
*** 2166,2175 ****
  	PyTypeObject *base;
  	int i, n;
  
! 	if (type->tp_flags & Py_TPFLAGS_READY) {
! 		assert(type->tp_dict != NULL);
  		return 0;
- 	}
  	assert((type->tp_flags & Py_TPFLAGS_READYING) == 0);
  
  	type->tp_flags |= Py_TPFLAGS_READYING;
--- 2161,2168 ----
  	PyTypeObject *base;
  	int i, n;
  
! 	if (type->tp_flags & Py_TPFLAGS_READY)
  		return 0;
  	assert((type->tp_flags & Py_TPFLAGS_READYING) == 0);
  
  	type->tp_flags |= Py_TPFLAGS_READYING;

--Guido van Rossum (home page: http://www.python.org/~guido/)