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.
This commit is contained in:
Phillip J. Eby 2006-04-15 01:02:17 +00:00
parent 3cfea2dc98
commit 8ebb28df3a
3 changed files with 22 additions and 26 deletions

View File

@ -421,7 +421,6 @@ Subject: Re: PEP 255: Simple Generators
... self.name = name ... self.name = name
... self.parent = None ... self.parent = None
... self.generator = self.generate() ... self.generator = self.generate()
... self.close = self.generator.close
... ...
... def generate(self): ... def generate(self):
... while not self.parent: ... 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 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 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 ' # Emacs turd '
@ -593,7 +590,6 @@ arguments are iterable -- a LazyList is the same as a generator to times().
... def __init__(self, g): ... def __init__(self, g):
... self.sofar = [] ... self.sofar = []
... self.fetch = g.next ... self.fetch = g.next
... self.close = g.close
... ...
... def __getitem__(self, i): ... def __getitem__(self, i):
... sofar, fetch = self.sofar, self.fetch ... 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] [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] [400, 405, 432, 450, 480, 486, 500, 512, 540, 576, 600, 625, 640, 648, 675]
>>> m235.close()
Ye olde Fibonacci generator, LazyList style. Ye olde Fibonacci generator, LazyList style.
>>> def fibgen(a, b): >>> def fibgen(a, b):
@ -648,7 +642,6 @@ Ye olde Fibonacci generator, LazyList style.
>>> fib = LazyList(fibgen(1, 2)) >>> fib = LazyList(fibgen(1, 2))
>>> firstn(iter(fib), 17) >>> firstn(iter(fib), 17)
[1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 377, 610, 987, 1597, 2584] [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) Running after your tail with itertools.tee (new in version 2.4)

View File

@ -454,9 +454,15 @@ frame_traverse(PyFrameObject *f, visitproc visit, void *arg)
static void static void
frame_clear(PyFrameObject *f) frame_clear(PyFrameObject *f)
{ {
PyObject **fastlocals, **p; PyObject **fastlocals, **p, **oldtop;
int i, slots; 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); Py_XDECREF(f->f_exc_type);
f->f_exc_type = NULL; f->f_exc_type = NULL;
@ -473,17 +479,13 @@ frame_clear(PyFrameObject *f)
slots = f->f_nlocals + f->f_ncells + f->f_nfreevars; slots = f->f_nlocals + f->f_ncells + f->f_nfreevars;
fastlocals = f->f_localsplus; fastlocals = f->f_localsplus;
for (i = slots; --i >= 0; ++fastlocals) { for (i = slots; --i >= 0; ++fastlocals) {
if (*fastlocals != NULL) { Py_CLEAR(*fastlocals);
Py_XDECREF(*fastlocals);
*fastlocals = NULL;
}
} }
/* stack */ /* stack */
if (f->f_stacktop != NULL) { if (oldtop != NULL) {
for (p = f->f_valuestack; p < f->f_stacktop; p++) { for (p = f->f_valuestack; p < oldtop; p++) {
Py_XDECREF(*p); Py_CLEAR(*p);
*p = NULL;
} }
} }
} }

View File

@ -35,7 +35,7 @@ gen_dealloc(PyGenObject *gen)
} }
_PyObject_GC_UNTRACK(self); _PyObject_GC_UNTRACK(self);
Py_XDECREF(gen->gi_frame); Py_CLEAR(gen->gi_frame);
PyObject_GC_Del(gen); PyObject_GC_Del(gen);
} }
@ -130,8 +130,8 @@ gen_close(PyGenObject *gen, PyObject *args)
"generator ignored GeneratorExit"); "generator ignored GeneratorExit");
return NULL; return NULL;
} }
if ( PyErr_ExceptionMatches(PyExc_StopIteration) if ( PyErr_ExceptionMatches(PyExc_StopIteration)
|| PyErr_ExceptionMatches(PyExc_GeneratorExit) ) || PyErr_ExceptionMatches(PyExc_GeneratorExit) )
{ {
PyErr_Clear(); /* ignore these errors */ PyErr_Clear(); /* ignore these errors */
Py_INCREF(Py_None); Py_INCREF(Py_None);
@ -208,7 +208,7 @@ PyDoc_STRVAR(throw_doc,
return next yielded value or raise StopIteration."); return next yielded value or raise StopIteration.");
static PyObject * static PyObject *
gen_throw(PyGenObject *gen, PyObject *args) gen_throw(PyGenObject *gen, PyObject *args)
{ {
PyObject *typ; PyObject *typ;
PyObject *tb = NULL; PyObject *tb = NULL;
@ -328,7 +328,7 @@ PyTypeObject PyGen_Type = {
0, /* tp_getset */ 0, /* tp_getset */
0, /* tp_base */ 0, /* tp_base */
0, /* tp_dict */ 0, /* tp_dict */
0, /* tp_descr_get */ 0, /* tp_descr_get */
0, /* tp_descr_set */ 0, /* tp_descr_set */
0, /* tp_dictoffset */ 0, /* tp_dictoffset */
@ -366,15 +366,16 @@ PyGen_NeedsFinalizing(PyGenObject *gen)
int i; int i;
PyFrameObject *f = gen->gi_frame; PyFrameObject *f = gen->gi_frame;
if (f == NULL || f->f_stacktop==NULL || f->f_iblock<=0) if (f == NULL || f->f_stacktop == NULL || f->f_iblock <= 0)
return 0; /* no frame or no blockstack == no finalization */ 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) if (f->f_blockstack[i].b_type != SETUP_LOOP)
/* any block type besides a loop requires cleanup */
return 1; return 1;
} }
/* No blocks except loops, it's safe to skip finalization */ /* No blocks except loops, it's safe to skip finalization. */
return 0; return 0;
} }