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

Move exception state information from frame objects to coroutine (generator/thread) object where it belongs.

Author: markshannon

Include/frameobject.h

@@ -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;
 

Include/genobject.h

@@ -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. */

Include/pyerrors.h

@@ -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,

Include/pystate.h

@@ -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 */
 
@@ -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 */
 

Lib/test/test_exceptions.py

@@ -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):
 

Lib/test/test_sys.py

@@ -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'))
@@ -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

Misc/NEWS.d/next/C API/2017-10-22-13-12-28.bpo-25612.1jnWKT.rst

@@ -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.

Objects/frameobject.c

@@ -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.
@@ -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)
@@ -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);
@@ -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 */
@@ -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;

Objects/genobject.c

@@ -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
+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
@@ -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)
 {
@@ -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);
 }
 
@@ -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
@@ -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);
@@ -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