[Python-checkins] python/dist/src/Modules gcmodule.c,2.72,2.73

[tim_one at users.sourceforge.net]

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

Modified Files:
	gcmodule.c 
Log Message:
update_refs():  assert that incoming refcounts aren't 0.  The comment
for this function has always claimed that was true, but it wasn't
verified before.  For the latest batch of "double deallocation" bugs
(stemming from weakref callbacks invoked by way of subtype_dealloc),
this assert would have triggered (instead of waiting for
_Py_ForgetReference to die with a segfault later).


Index: gcmodule.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Modules/gcmodule.c,v
retrieving revision 2.72
retrieving revision 2.73
diff -C2 -d -r2.72 -r2.73
*** gcmodule.c	4 Sep 2003 11:59:50 -0000	2.72
--- gcmodule.c	14 Nov 2003 00:01:17 -0000	2.73
***************
*** 215,218 ****
--- 215,237 ----
  		assert(gc->gc.gc_refs == GC_REACHABLE);
  		gc->gc.gc_refs = FROM_GC(gc)->ob_refcnt;
+ 		/* Python's cyclic gc should never see an incoming refcount
+ 		 * of 0:  if something decref'ed to 0, it should have been
+ 		 * deallocated immediately at that time.
+ 		 * Possible cause (if the assert triggers):  a tp_dealloc
+ 		 * routine left a gc-aware object tracked during its teardown
+ 		 * phase, and did something-- or allowed something to happen --
+ 		 * that called back into Python.  gc can trigger then, and may
+ 		 * see the still-tracked dying object.  Before this assert
+ 		 * was added, such mistakes went on to allow gc to try to
+ 		 * delete the object again.  In a debug build, that caused
+ 		 * a mysterious segfault, when _Py_ForgetReference tried
+ 		 * to remove the object from the doubly-linked list of all
+ 		 * objects a second time.  In a release build, an actual
+ 		 * double deallocation occurred, which leads to corruption
+ 		 * of the allocator's internal bookkeeping pointers.  That's
+ 		 * so serious that maybe this should be a release-build
+ 		 * check instead of an assert?
+ 		 */
+ 		assert(gc->gc.gc_refs != 0);
  	}
  }