[Python-checkins] python/dist/src/Objects typeobject.c,2.149,2.150

[gvanrossum@users.sourceforge.net]

Update of /cvsroot/python/python/dist/src/Objects
In directory usw-pr-cvs1:/tmp/cvs-serv26343

Modified Files:
	typeobject.c 
Log Message:
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.


Index: typeobject.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Objects/typeobject.c,v
retrieving revision 2.149
retrieving revision 2.150
diff -C2 -d -r2.149 -r2.150
*** typeobject.c	10 Jun 2002 14:30:43 -0000	2.149
--- typeobject.c	10 Jun 2002 15:24:42 -0000	2.150
***************
*** 291,294 ****
--- 291,303 ----
  	}
  
+ 	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);
***************
*** 333,342 ****
  	}
  
! 	if (type->tp_dictoffset != base->tp_dictoffset) {
! 		PyObject **dictptr = _PyObject_GetDictPtr(self);
! 		if (dictptr && *dictptr) {
! 			PyDict_Clear(*dictptr);
! 		}
! 	}
  
  	if (baseclear)
--- 342,347 ----
  	}
  
! 	/* There's no need to clear the instance dict (if any);
! 	   the collector will call its tp_clear handler. */
  
  	if (baseclear)
***************
*** 1484,1494 ****
  type_traverse(PyTypeObject *type, visitproc visit, void *arg)
  {
- 	etype *et;
  	int err;
  
! 	if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE))
! 		return 0;
! 
! 	et = (etype *)type;
  
  #define VISIT(SLOT) \
--- 1489,1497 ----
  type_traverse(PyTypeObject *type, visitproc visit, void *arg)
  {
  	int err;
  
! 	/* Because of type_is_gc(), the collector only calls this
! 	   for heaptypes. */
! 	assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE);
  
  #define VISIT(SLOT) \
***************
*** 1504,1509 ****
  	VISIT(type->tp_bases);
  	VISIT(type->tp_base);
! 	VISIT(type->tp_subclasses);
! 	VISIT(et->slots);
  
  #undef VISIT
--- 1507,1515 ----
  	VISIT(type->tp_bases);
  	VISIT(type->tp_base);
! 
! 	/* 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
***************
*** 1515,1525 ****
  type_clear(PyTypeObject *type)
  {
- 	etype *et;
  	PyObject *tmp;
  
! 	if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE))
! 		return 0;
! 
! 	et = (etype *)type;
  
  #define CLEAR(SLOT) \
--- 1521,1529 ----
  type_clear(PyTypeObject *type)
  {
  	PyObject *tmp;
  
! 	/* Because of type_is_gc(), the collector only calls this
! 	   for heaptypes. */
! 	assert(type->tp_flags & Py_TPFLAGS_HEAPTYPE);
  
  #define CLEAR(SLOT) \
***************
*** 1530,1545 ****
  	}
  
! 	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
--- 1534,1563 ----
  	}
  
! 	/* 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:
  
! 	   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
***************
*** 2158,2165 ****
  	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);
  
--- 2176,2181 ----
  	int i, n;
  
! 	if (type->tp_flags & Py_TPFLAGS_READY)
  		return 0;
  	assert((type->tp_flags & Py_TPFLAGS_READYING) == 0);