[ python-Bugs-1470508 ] Error in PyGen_NeedsFinalizing()

[SourceForge.net]

Bugs item #1470508, was opened at 2006-04-14 15:31
Message generated for change (Comment added) made by tim_one
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1470508&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Python Interpreter Core
Group: Python 2.5
>Status: Closed
>Resolution: Fixed
Priority: 7
Submitted By: Tim Peters (tim_one)
>Assigned to: Nobody/Anonymous (nobody)
Summary: Error in PyGen_NeedsFinalizing()

Initial Comment:
There's an off-by-one error in PyGen_NeedsFinalizing():
 the loop starts by looking at
f->f_blockstack[f->f_iblock], but that's one beyond the
largest legitimate f_blockstack index.  As a result,
any generator with an active loop is very likely to
claim it needs finalizing (to the extent that it's
unlikely that reading up trash for b_type will match
SETUP_LOOP).

The attached patch repairs that, but also tries to
remove some of the goofy .close() gimmicks added to old
tests in test_generators to worm around "leaks" caused
by that all generators said they needed finalizers in
2.5 at first.  (That's how I bumped into this:  I tried
removing the .close() gimmicks first, but -R:: runs
said they still leaked -- that started the trail
leading to PyGen_NeedsFinalizing()).

Alas, after applying the patch, test_generators craps
out in a debug build with a NULL-pointer dereference in
_Py_ForgetReference(), in the guts of the `if` test
deciding whether to produce an "UNREF invalid object"
error.  At the time, op_type is _PyGen_Type and
op->_ob_next = _ob_prev = NULL, probably meaning that
op was already passed to _Py_ForgetReference() previously.

We get into _Py_ForgetReference() via a chain starting
at the the decref in ceval.c's

	while (STACK_LEVEL() > b->b_level) {
		v = POP();
		Py_XDECREF(v);

and we got into PyEval_EvalFrameEx via gen_send_ex via
gen_close via gen_dealloc via (eventually) a dict on
some instance going away ... and then I got lost. 
Looks like there are several more layers of generator
deallocs on the call stack too.

I'm out of time for looking at this now, so recording
it here.

----------------------------------------------------------------------

>Comment By: Tim Peters (tim_one)
Date: 2006-04-14 22:28

Message:
Logged In: YES 
user_id=31435

Phillip checked in a related patch that looked good, fixed
the segfaults, and allowed to get rid of the explicit closes
in the LazyList-based examples without new leaking.  (The
tee-based examples still need explicit closing to avoid
leaking, and I added XXX comments about that.)  So, closing
as fixed.

Thanks!

----------------------------------------------------------------------

Comment By: Thomas Wouters (twouters)
Date: 2006-04-14 19:22

Message:
Logged In: YES 
user_id=34209

As explained on python-dev, I believe this patch fixes the
crash. It still leaves test_generators leaking 255
references, though, which I find completely unsurprising
given the pain it's caused so far :) I do believe these
changes are correct even if we don't fix the generator
leakage, but someone with more fundamental knowledge of the
cyclic GC might want to pronounce.


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1470508&group_id=5470