From 8ebb28df3a6e0bce240b6c2aa20d7aa5a4dfef39 Mon Sep 17 00:00:00 2001 From: "Phillip J. Eby" Date: Sat, 15 Apr 2006 01:02:17 +0000 Subject: [PATCH] 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. --- Lib/test/test_generators.py | 7 ------- Objects/frameobject.c | 20 +++++++++++--------- Objects/genobject.c | 21 +++++++++++---------- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 3b7933ea289..b44d63788f1 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 8aa33771801..49f74cb2099 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 ccce315ee91..c73a53cb12b 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; }