[Patches] Trashcan vs. GC vs. TRACE_REFS

[Charles G Waldman]

Hi Christian et al.

Well, I said there was nothing wrong with Trashcan, but I spoke a
little too soon ;-)

With the current gc-cycle patch and Py_TRACE_REFS set, we get a core
dump.

The culprit is this little piece of code in object.c

void
_Py_Dealloc(op)
	PyObject *op;
{
	destructor dealloc = op->ob_type->tp_dealloc;
	_Py_ForgetReference(op);
***	if (_PyTrash_delete_nesting < PyTrash_UNWIND_LEVEL-1)
***	    op->ob_type = NULL; 
	(*dealloc)(op);
}


I'd really love to understand what those lines marked "***" are for.

If you set ob_type to NULL before doing a dealloc, you get into
trouble, because the new GC dealloc functions need to check ob_type to
decide if the object being deleted has GC info.  By setting type to
NULL you are breaking this.

But what really seems odd to me is that the _Py_Dealloc function above
is only used when Py_TRACE_REFS is set.  Normally the following macro
definition is used instead (see object.h)

ifndef Py_TRACE_REFS
...
#define _Py_Dealloc(op) (*(op)->ob_type->tp_dealloc)((PyObject *)(op))
...
#endif /* !Py_TRACE_REFS */

So, what is the point of worrying about _PyTrash_UNWIND_LEVEL only in
a function which is only used in special builds?  If this is really
important, I'd think you'd be doing it in the _Py_Dealloc macro as
well.  But, of course, you can't be clobbering ob_type at all, because
the gc dealloc functions *need* that ob_type.

As far as I can tell, building with Py_TRACE_REFS is not too widely
done.  But I'm finding it incredibly useful for tracking down those
reference leaks which still remain even with Neil's GC patches in
place.