[Python-checkins] python/dist/src/Objects typeobject.c,2.251,2.252

[tim_one at users.sourceforge.net]

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

Modified Files:
	typeobject.c 
Log Message:
subtype_dealloc():  A more complete fix for critical bug 840829 +
expanded the test case with a piece that needs the more-complete fix.

I'll backport this to 2.3 maint.


Index: typeobject.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Objects/typeobject.c,v
retrieving revision 2.251
retrieving revision 2.252
diff -C2 -d -r2.251 -r2.252
*** typeobject.c	12 Nov 2003 20:43:28 -0000	2.251
--- typeobject.c	13 Nov 2003 21:59:32 -0000	2.252
***************
*** 640,647 ****
  	Py_TRASHCAN_SAFE_BEGIN(self);
  	--_PyTrash_delete_nesting;
! 	/* DO NOT restore GC tracking at this point.  The weakref callback
! 	 * (if any) may trigger GC, and if self is tracked at that point,
! 	 * it will look like trash to GC and GC will try to delete it
! 	 * again.  Double-deallocation is a subtle disaster.
  	 */
  
--- 640,647 ----
  	Py_TRASHCAN_SAFE_BEGIN(self);
  	--_PyTrash_delete_nesting;
! 	/* DO NOT restore GC tracking at this point.  weakref callbacks
! 	 * (if any, and whether directly here or indirectly in something we
! 	 * call) may trigger GC, and if self is tracked at that point, it
! 	 * will look like trash to GC and GC will try to delete self again.
  	 */
  
***************
*** 659,669 ****
  	if (type->tp_weaklistoffset && !base->tp_weaklistoffset)
  		PyObject_ClearWeakRefs(self);
- 	_PyObject_GC_TRACK(self); /* We'll untrack for real later */
  
  	/* Maybe call finalizer; exit early if resurrected */
  	if (type->tp_del) {
  		type->tp_del(self);
  		if (self->ob_refcnt > 0)
! 			goto endlabel;
  	}
  
--- 659,671 ----
  	if (type->tp_weaklistoffset && !base->tp_weaklistoffset)
  		PyObject_ClearWeakRefs(self);
  
  	/* Maybe call finalizer; exit early if resurrected */
  	if (type->tp_del) {
+ 		_PyObject_GC_TRACK(self);
  		type->tp_del(self);
  		if (self->ob_refcnt > 0)
! 			goto endlabel;	/* resurrected */
! 		else
! 			_PyObject_GC_UNTRACK(self);
  	}
  
***************
*** 690,693 ****
--- 692,696 ----
  
  	/* Finalize GC if the base doesn't do GC and we do */
+ 	_PyObject_GC_TRACK(self);
  	if (!PyType_IS_GC(base))
  		_PyObject_GC_UNTRACK(self);
***************
*** 730,733 ****
--- 733,746 ----
  		 trashcan begin
  		 GC track
+ 
+            Q. Why did the last question say "immediately GC-track again"?
+               It's nowhere near immediately.
+ 
+            A. Because the code *used* to re-track immediately.  Bad Idea.
+               self has a refcount of 0, and if gc ever gets its hands on it
+               (which can happen if any weakref callback gets invoked), it
+               looks like trash to gc too, and gc also tries to delete self
+               then.  But we're already deleting self.  Double dealloction is
+               a subtle disaster.
  
  	   Q. Why the bizarre (net-zero) manipulation of