Skip to content

Move exc state to generator. Fixes bpo-25612 #1773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Oct 22, 2017
8 changes: 0 additions & 8 deletions Include/frameobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,6 @@ typedef struct _frame {
char f_trace_lines; /* Emit per-line trace events? */
char f_trace_opcodes; /* Emit per-opcode trace events? */

/* In a generator, we need to be able to swap between the exception
state inside the generator and the exception state of the calling
frame (which shouldn't be impacted when the generator "yields"
from an except handler).
These three fields exist exactly for that, and are unused for
non-generator frames. See the save_exc_state and swap_exc_state
functions in ceval.c for details of their use. */
PyObject *f_exc_type, *f_exc_value, *f_exc_traceback;
/* Borrowed reference to a generator, or NULL */
PyObject *f_gen;

Expand Down
3 changes: 2 additions & 1 deletion Include/genobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ struct _frame; /* Avoid including frameobject.h */
/* Name of the generator. */ \
PyObject *prefix##_name; \
/* Qualified name of the generator. */ \
PyObject *prefix##_qualname;
PyObject *prefix##_qualname; \
_PyErr_StackItem prefix##_exc_state;

typedef struct {
/* The gi_ prefix is intended to remind of generator-iterator. */
Expand Down
1 change: 1 addition & 0 deletions Include/pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ PyAPI_FUNC(void) PyErr_SetNone(PyObject *);
PyAPI_FUNC(void) PyErr_SetObject(PyObject *, PyObject *);
#ifndef Py_LIMITED_API
PyAPI_FUNC(void) _PyErr_SetKeyError(PyObject *);
_PyErr_StackItem *_PyErr_GetTopmostException(PyThreadState *tstate);
#endif
PyAPI_FUNC(void) PyErr_SetString(
PyObject *exception,
Expand Down
27 changes: 24 additions & 3 deletions Include/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,21 @@ typedef int (*Py_tracefunc)(PyObject *, struct _frame *, int, PyObject *);
#ifdef Py_LIMITED_API
typedef struct _ts PyThreadState;
#else

typedef struct _err_stackitem {
/* This struct represents an entry on the exception stack, which is a
* per-coroutine state. (Coroutine in the computer science sense,
* including the thread and generators).
* This ensures that the exception state is not impacted by "yields"
* from an except handler.
*/
PyObject *exc_type, *exc_value, *exc_traceback;

struct _err_stackitem *previous_item;

} _PyErr_StackItem;


typedef struct _ts {
/* See Python/ceval.c for comments explaining most fields */

Expand All @@ -147,13 +162,19 @@ typedef struct _ts {
PyObject *c_profileobj;
PyObject *c_traceobj;

/* The exception currently being raised */
PyObject *curexc_type;
PyObject *curexc_value;
PyObject *curexc_traceback;

PyObject *exc_type;
PyObject *exc_value;
PyObject *exc_traceback;
/* The exception currently being handled, if no coroutines/generators
* are present. Always last element on the stack referred to be exc_info.
*/
_PyErr_StackItem exc_state;

/* Pointer to the top of the stack of the exceptions currently
* being handled */
_PyErr_StackItem *exc_info;

PyObject *dict; /* Stores per-thread state */

Expand Down
56 changes: 56 additions & 0 deletions Lib/test/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,62 @@ def test_unhandled(self):
self.assertIn("test message", report)
self.assertTrue(report.endswith("\n"))

def test_yield_in_nested_try_excepts(self):
#Issue #25612
class MainError(Exception):
pass

class SubError(Exception):
pass

def main():
try:
raise MainError()
except MainError:
try:
yield
except SubError:
pass
raise

coro = main()
coro.send(None)
with self.assertRaises(MainError):
coro.throw(SubError())

def test_generator_doesnt_retain_old_exc2(self):
#Issue 28884#msg282532
def g():
try:
raise ValueError
except ValueError:
yield 1
self.assertEqual(sys.exc_info(), (None, None, None))
yield 2

gen = g()

try:
raise IndexError
except IndexError:
self.assertEqual(next(gen), 1)
self.assertEqual(next(gen), 2)

def test_raise_in_generator(self):
#Issue 25612#msg304117
def g():
yield 1
raise
yield 2

with self.assertRaises(ZeroDivisionError):
i = g()
try:
1/0
except:
next(i)
next(i)


class ImportErrorTests(unittest.TestCase):

Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,7 @@ class C(object): pass
nfrees = len(x.f_code.co_freevars)
extras = x.f_code.co_stacksize + x.f_code.co_nlocals +\
ncells + nfrees - 1
check(x, vsize('8P2c4P3ic' + CO_MAXBLOCKS*'3i' + 'P' + extras*'P'))
check(x, vsize('5P2c4P3ic' + CO_MAXBLOCKS*'3i' + 'P' + extras*'P'))
# function
def func(): pass
check(func, size('12P'))
Expand All @@ -988,7 +988,7 @@ def bar(cls):
check(bar, size('PP'))
# generator
def get_gen(): yield 1
check(get_gen(), size('Pb2PPP'))
check(get_gen(), size('Pb2PPP4P'))
# iterator
check(iter('abc'), size('lP'))
# callable-iterator
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Move the current exception state from the frame object to the co-routine.
This simplifies the interpreter and fixes a couple of obscure bugs caused by
having swap exception state when entering or exiting a generator.
13 changes: 1 addition & 12 deletions Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,7 @@ static PyGetSetDef frame_getsetlist[] = {

* ob_type, ob_size, f_code, f_valuestack;

* f_locals, f_trace,
f_exc_type, f_exc_value, f_exc_traceback are NULL;
* f_locals, f_trace are NULL;

* f_localsplus does not require re-allocation and
the local variables in f_localsplus are NULL.
Expand Down Expand Up @@ -438,9 +437,6 @@ frame_dealloc(PyFrameObject *f)
Py_DECREF(f->f_globals);
Py_CLEAR(f->f_locals);
Py_CLEAR(f->f_trace);
Py_CLEAR(f->f_exc_type);
Py_CLEAR(f->f_exc_value);
Py_CLEAR(f->f_exc_traceback);

co = f->f_code;
if (co->co_zombieframe == NULL)
Expand Down Expand Up @@ -469,9 +465,6 @@ frame_traverse(PyFrameObject *f, visitproc visit, void *arg)
Py_VISIT(f->f_globals);
Py_VISIT(f->f_locals);
Py_VISIT(f->f_trace);
Py_VISIT(f->f_exc_type);
Py_VISIT(f->f_exc_value);
Py_VISIT(f->f_exc_traceback);

/* locals */
slots = f->f_code->co_nlocals + PyTuple_GET_SIZE(f->f_code->co_cellvars) + PyTuple_GET_SIZE(f->f_code->co_freevars);
Expand Down Expand Up @@ -502,9 +495,6 @@ frame_tp_clear(PyFrameObject *f)
f->f_stacktop = NULL;
f->f_executing = 0;

Py_CLEAR(f->f_exc_type);
Py_CLEAR(f->f_exc_value);
Py_CLEAR(f->f_exc_traceback);
Py_CLEAR(f->f_trace);

/* locals */
Expand Down Expand Up @@ -698,7 +688,6 @@ _PyFrame_New_NoTrack(PyThreadState *tstate, PyCodeObject *code,
f->f_localsplus[i] = NULL;
f->f_locals = NULL;
f->f_trace = NULL;
f->f_exc_type = f->f_exc_value = f->f_exc_traceback = NULL;
}
f->f_stacktop = f->f_valuestack;
f->f_builtins = builtins;
Expand Down
46 changes: 35 additions & 11 deletions Objects/genobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,23 @@ static char *NON_INIT_CORO_MSG = "can't send non-None value to a "
static char *ASYNC_GEN_IGNORED_EXIT_MSG =
"async generator ignored GeneratorExit";

static inline int
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Py_LOCAL_INLINE(int)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we support C99, I think we ought to use standard C where possible.

exc_state_traverse(_PyErr_StackItem *exc_state, visitproc visit, void *arg)
{
Py_VISIT(exc_state->exc_type);
Py_VISIT(exc_state->exc_value);
Py_VISIT(exc_state->exc_traceback);
return 0;
}

static int
gen_traverse(PyGenObject *gen, visitproc visit, void *arg)
{
Py_VISIT((PyObject *)gen->gi_frame);
Py_VISIT(gen->gi_code);
Py_VISIT(gen->gi_name);
Py_VISIT(gen->gi_qualname);
return 0;
return exc_state_traverse(&gen->gi_exc_state, visit, arg);
}

void
Expand Down Expand Up @@ -87,6 +96,21 @@ _PyGen_Finalize(PyObject *self)
PyErr_Restore(error_type, error_value, error_traceback);
}

static inline void
exc_state_clear(_PyErr_StackItem *exc_state)
{
PyObject *t, *v, *tb;
t = exc_state->exc_type;
v = exc_state->exc_value;
tb = exc_state->exc_traceback;
exc_state->exc_type = NULL;
exc_state->exc_value = NULL;
exc_state->exc_traceback = NULL;
Py_XDECREF(t);
Py_XDECREF(v);
Py_XDECREF(tb);
}

static void
gen_dealloc(PyGenObject *gen)
{
Expand Down Expand Up @@ -116,6 +140,7 @@ gen_dealloc(PyGenObject *gen)
Py_CLEAR(gen->gi_code);
Py_CLEAR(gen->gi_name);
Py_CLEAR(gen->gi_qualname);
exc_state_clear(&gen->gi_exc_state);
PyObject_GC_Del(gen);
}

Expand Down Expand Up @@ -187,7 +212,11 @@ gen_send_ex(PyGenObject *gen, PyObject *arg, int exc, int closing)
f->f_back = tstate->frame;

gen->gi_running = 1;
gen->gi_exc_state.previous_item = tstate->exc_info;
tstate->exc_info = &gen->gi_exc_state;
result = PyEval_EvalFrameEx(f, exc);
tstate->exc_info = gen->gi_exc_state.previous_item;
gen->gi_exc_state.previous_item = NULL;
gen->gi_running = 0;

/* Don't keep the reference to f_back any longer than necessary. It
Expand Down Expand Up @@ -281,16 +310,7 @@ gen_send_ex(PyGenObject *gen, PyObject *arg, int exc, int closing)
if (!result || f->f_stacktop == NULL) {
/* generator can't be rerun, so release the frame */
/* first clean reference cycle through stored exception traceback */
PyObject *t, *v, *tb;
t = f->f_exc_type;
v = f->f_exc_value;
tb = f->f_exc_traceback;
f->f_exc_type = NULL;
f->f_exc_value = NULL;
f->f_exc_traceback = NULL;
Py_XDECREF(t);
Py_XDECREF(v);
Py_XDECREF(tb);
exc_state_clear(&gen->gi_exc_state);
gen->gi_frame->f_gen = NULL;
gen->gi_frame = NULL;
Py_DECREF(f);
Expand Down Expand Up @@ -810,6 +830,10 @@ gen_new_with_qualname(PyTypeObject *type, PyFrameObject *f,
gen->gi_code = (PyObject *)(f->f_code);
gen->gi_running = 0;
gen->gi_weakreflist = NULL;
gen->gi_exc_state.exc_type = NULL;
gen->gi_exc_state.exc_value = NULL;
gen->gi_exc_state.exc_traceback = NULL;
gen->gi_exc_state.previous_item = NULL;
if (name != NULL)
gen->gi_name = name;
else
Expand Down
Loading