Fix SF#1470508: crash in generator cycle finalization. There were two

problems: first, PyGen_NeedsFinalizing() had an off-by-one bug that
prevented it from ever saying a generator didn't need finalizing, and
second, frame objects cleared themselves in a way that caused their
owning generator to think they were still executable, causing a double
deallocation of objects on the value stack if there was still a loop
on the block stack.  This revision also removes some unnecessary
close() operations from test_generators that are now appropriately
handled by the cycle collector.

3 files changed, 22 insertions, 26 deletions

diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py
index 3b7933e..b44d637 100644
--- a/Lib/test/test_generators.py
+++ b/Lib/test/test_generators.py
@@ -421,7 +421,6 @@ Subject: Re: PEP 255: Simple Generators
 ...         self.name = name
 ...         self.parent = None
 ...         self.generator = self.generate()
-...         self.close = self.generator.close
 ...
 ...     def generate(self):
 ...         while not self.parent:
@@ -484,8 +483,6 @@ A->A B->G C->A D->G E->G F->A G->G H->G I->A J->G K->A L->A M->G
 merged A into G
 A->G B->G C->G D->G E->G F->G G->G H->G I->G J->G K->G L->G M->G
 
->>> for s in sets: s.close()    # break cycles
-
 """
 # Emacs turd '
 
@@ -593,7 +590,6 @@ arguments are iterable -- a LazyList is the same as a generator to times().
 ...     def __init__(self, g):
 ...         self.sofar = []
 ...         self.fetch = g.next
-...         self.close = g.close
 ...
 ...     def __getitem__(self, i):
 ...         sofar, fetch = self.sofar, self.fetch
@@ -624,8 +620,6 @@ efficient.
 [200, 216, 225, 240, 243, 250, 256, 270, 288, 300, 320, 324, 360, 375, 384]
 [400, 405, 432, 450, 480, 486, 500, 512, 540, 576, 600, 625, 640, 648, 675]
 
->>> m235.close()
-
 Ye olde Fibonacci generator, LazyList style.
 
 >>> def fibgen(a, b):
@@ -648,7 +642,6 @@ Ye olde Fibonacci generator, LazyList style.
 >>> fib = LazyList(fibgen(1, 2))
 >>> firstn(iter(fib), 17)
 [1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 377, 610, 987, 1597, 2584]
->>> fib.close()
 
 
 Running after your tail with itertools.tee (new in version 2.4)
diff --git a/Objects/frameobject.c b/Objects/frameobject.c
index 8aa3377..49f74cb 100644
--- a/Objects/frameobject.c
+++ b/Objects/frameobject.c
@@ -454,9 +454,15 @@ frame_traverse(PyFrameObject *f, visitproc visit, void *arg)
 static void
 frame_clear(PyFrameObject *f)
 {
-	PyObject **fastlocals, **p;
+	PyObject **fastlocals, **p, **oldtop;
 	int i, slots;
 
+	oldtop = f->f_stacktop;
+
+	/* Before anything else, make sure that this frame is clearly marked 
+           as being defunct! */
+        f->f_stacktop = NULL;
+
 	Py_XDECREF(f->f_exc_type);
 	f->f_exc_type = NULL;
 
@@ -473,17 +479,13 @@ frame_clear(PyFrameObject *f)
 	slots = f->f_nlocals + f->f_ncells + f->f_nfreevars;
 	fastlocals = f->f_localsplus;
 	for (i = slots; --i >= 0; ++fastlocals) {
-		if (*fastlocals != NULL) {
-			Py_XDECREF(*fastlocals);
-			*fastlocals = NULL;
-		}
+		Py_CLEAR(*fastlocals);
 	}
 
 	/* stack */
-	if (f->f_stacktop != NULL) {
-		for (p = f->f_valuestack; p < f->f_stacktop; p++) {
-			Py_XDECREF(*p);
-			*p = NULL;
+	if (oldtop != NULL) {
+		for (p = f->f_valuestack; p < oldtop; p++) {
+			Py_CLEAR(*p);
 		}
 	}
 }
diff --git a/Objects/genobject.c b/Objects/genobject.c
index ccce315..c73a53cb 100644
--- a/Objects/genobject.c
+++ b/Objects/genobject.c
@@ -35,7 +35,7 @@ gen_dealloc(PyGenObject *gen)
 	}
 
 	_PyObject_GC_UNTRACK(self);
-	Py_XDECREF(gen->gi_frame);
+	Py_CLEAR(gen->gi_frame);
 	PyObject_GC_Del(gen);
 }
 
@@ -130,8 +130,8 @@ gen_close(PyGenObject *gen, PyObject *args)
 			"generator ignored GeneratorExit");
 		return NULL;
 	}
-	if ( PyErr_ExceptionMatches(PyExc_StopIteration) 
-	     || PyErr_ExceptionMatches(PyExc_GeneratorExit) ) 
+	if ( PyErr_ExceptionMatches(PyExc_StopIteration)
+	     || PyErr_ExceptionMatches(PyExc_GeneratorExit) )
 	{
 		PyErr_Clear();	/* ignore these errors */
 		Py_INCREF(Py_None);
@@ -208,7 +208,7 @@ PyDoc_STRVAR(throw_doc,
 return next yielded value or raise StopIteration.");
 
 static PyObject *
-gen_throw(PyGenObject *gen, PyObject *args) 
+gen_throw(PyGenObject *gen, PyObject *args)
 {
 	PyObject *typ;
 	PyObject *tb = NULL;
@@ -328,7 +328,7 @@ PyTypeObject PyGen_Type = {
 	0,					/* tp_getset */
 	0,					/* tp_base */
 	0,					/* tp_dict */
-        
+
 	0,					/* tp_descr_get */
 	0,					/* tp_descr_set */
 	0,					/* tp_dictoffset */
@@ -366,15 +366,16 @@ PyGen_NeedsFinalizing(PyGenObject *gen)
 	int i;
 	PyFrameObject *f = gen->gi_frame;
 
-	if (f == NULL || f->f_stacktop==NULL || f->f_iblock<=0)
-		return 0; /* no frame or no blockstack == no finalization */
+	if (f == NULL || f->f_stacktop == NULL || f->f_iblock <= 0)
+		return 0; /* no frame or empty blockstack == no finalization */
 
-	for (i=f->f_iblock; i>=0; i--) {
+	/* Any block type besides a loop requires cleanup. */
+	i = f->f_iblock;
+	while (--i >= 0) {
 		if (f->f_blockstack[i].b_type != SETUP_LOOP)
-			/* any block type besides a loop requires cleanup */
 			return 1;
 	}
 
-	/* No blocks except loops, it's safe to skip finalization */
+	/* No blocks except loops, it's safe to skip finalization. */
 	return 0;
 }