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.

diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py
index 3b7933ea2895decc1f80e2203a2156d49eef8db8..b44d63788f14d68f5c0889f17dbdff541af184a2 100644 (file)
--- 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 8aa33771801b776f9912923291b451c7450c3011..49f74cb20996194b54279b303b55c14c76fef60c 100644 (file)
--- 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 ccce315ee910be63181a83570a10a6c80d31ee85..c73a53cb12b1fad10dcbc111d8bf9a1f8a86479d 100644 (file)
--- 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;
 }