Improve handling of exception state in coroutines to prevent swallowing exceptions of the caller.

See #1731.

4 files changed, 152 insertions, 29 deletions

diff --git a/Cython/Utility/Coroutine.c b/Cython/Utility/Coroutine.c
index b7b654687..a83955101 100644
--- a/Cython/Utility/Coroutine.c
+++ b/Cython/Utility/Coroutine.c
@@ -434,6 +434,7 @@ static int __pyx_Generator_init(void); /*proto*/
 //@requires: Exceptions.c::PyThreadStateGet
 //@requires: Exceptions.c::SwapException
 //@requires: Exceptions.c::RaiseException
+//@requires: Exceptions.c::SaveResetException
 //@requires: ObjectHandling.c::PyObjectCallMethod1
 //@requires: ObjectHandling.c::PyObjectGetAttrStr
 //@requires: CommonStructures.c::FetchCommonType
@@ -618,8 +619,9 @@ static void __Pyx__Coroutine_AlreadyTerminatedError(CYTHON_UNUSED PyObject *gen,
 
 static
 PyObject *__Pyx_Coroutine_SendEx(__pyx_CoroutineObject *self, PyObject *value, int closing) {
-    PyObject *retval;
     __Pyx_PyThreadState_declare
+    PyObject *retval;
+    int restore_exc;
 
     assert(!self->is_running);
 
@@ -634,7 +636,7 @@ PyObject *__Pyx_Coroutine_SendEx(__pyx_CoroutineObject *self, PyObject *value, i
     }
 
     __Pyx_PyThreadState_assign
-    if (value) {
+    if (self->exc_type && self->exc_type != Py_None) {
 #if CYTHON_COMPILING_IN_PYPY || CYTHON_COMPILING_IN_PYSTON
         // FIXME: what to do in PyPy?
 #else
@@ -649,10 +651,18 @@ PyObject *__Pyx_Coroutine_SendEx(__pyx_CoroutineObject *self, PyObject *value, i
             f->f_back = $local_tstate_cname->frame;
         }
 #endif
+        // We were in an except handler when we left,
+        // restore the exception state which was put aside.
         __Pyx_ExceptionSwap(&self->exc_type, &self->exc_value,
                             &self->exc_traceback);
+        // self->exc_* now holds the exception state of the caller
+        restore_exc = 1;
     } else {
+        // save away the exception state of the caller
         __Pyx_Coroutine_ExceptionClear(self);
+        __Pyx_ExceptionSave(&self->exc_type, &self->exc_value,
+                            &self->exc_traceback);
+        restore_exc = 0;
     }
 
     self->is_running = 1;
@@ -660,21 +670,23 @@ PyObject *__Pyx_Coroutine_SendEx(__pyx_CoroutineObject *self, PyObject *value, i
     self->is_running = 0;
 
     if (retval) {
+        // restore exception state of caller and save the state of the coroutine
         __Pyx_ExceptionSwap(&self->exc_type, &self->exc_value,
                             &self->exc_traceback);
+    }
 #if CYTHON_COMPILING_IN_PYPY || CYTHON_COMPILING_IN_PYSTON
-        // FIXME: what to do in PyPy?
+    // FIXME: what to do in PyPy?
 #else
-        // Don't keep the reference to f_back any longer than necessary.  It
-        // may keep a chain of frames alive or it could create a reference
-        // cycle.
-        if (self->exc_traceback) {
-            PyTracebackObject *tb = (PyTracebackObject *) self->exc_traceback;
-            PyFrameObject *f = tb->tb_frame;
-            Py_CLEAR(f->f_back);
-        }
+    // Don't keep the reference to f_back any longer than necessary.  It
+    // may keep a chain of frames alive or it could create a reference
+    // cycle.
+    if (restore_exc && self->exc_traceback) {
+        PyTracebackObject *tb = (PyTracebackObject *) self->exc_traceback;
+        PyFrameObject *f = tb->tb_frame;
+        Py_CLEAR(f->f_back);
+    }
 #endif
-    } else {
+    if (!restore_exc) {
         __Pyx_Coroutine_ExceptionClear(self);
     }
 
diff --git a/tests/run/generator_frame_cycle.py b/tests/run/generator_frame_cycle.py
index 9647306cf..d912ff9cc 100644
--- a/tests/run/generator_frame_cycle.py
+++ b/tests/run/generator_frame_cycle.py
@@ -3,11 +3,6 @@
 
 import sys
 
-def _next(it):
-    if sys.version_info[0] >= 3:
-        return next(it)
-    else:
-        return it.next()
 
 def test_generator_frame_cycle():
     """
@@ -23,8 +18,39 @@ def test_generator_frame_cycle():
         finally:
             testit.append("I'm done")
     g = whoo()
-    _next(g)
+    next(g)
+
     # Frame object cycle
     eval('g.throw(ValueError)', {'g': g})
     del g
+
+    return tuple(testit)
+
+
+def test_generator_frame_cycle_with_outer_exc():
+    """
+    >>> test_generator_frame_cycle_with_outer_exc()
+    ("I'm done",)
+    """
+    testit = []
+    def whoo():
+        try:
+            yield
+        except:
+            yield
+        finally:
+            testit.append("I'm done")
+    g = whoo()
+    next(g)
+
+    try:
+        raise ValueError()
+    except ValueError as exc:
+        assert sys.exc_info()[1] is exc, sys.exc_info()
+        # Frame object cycle
+        eval('g.throw(ValueError)', {'g': g})
+        assert sys.exc_info()[1] is exc, sys.exc_info()
+        del g
+        assert sys.exc_info()[1] is exc, sys.exc_info()
+
     return tuple(testit)
diff --git a/tests/run/generators_Tgh1731.pyx b/tests/run/generators_Tgh1731.pyx
new file mode 100644
index 000000000..99cae90ca
--- /dev/null
+++ b/tests/run/generators_Tgh1731.pyx
@@ -0,0 +1,70 @@
+# mode: run
+# ticket: gh1731
+
+
+def cygen():
+    yield 1
+
+
+def test_from_cython(g):
+    """
+    >>> def pygen(): yield 1
+    >>> test_from_cython(pygen)
+    Traceback (most recent call last):
+    ZeroDivisionError: integer division or modulo by zero
+
+    >>> test_from_cython(cygen)
+    Traceback (most recent call last):
+    ZeroDivisionError: integer division or modulo by zero
+    """
+    try:
+        1 / 0
+    except:
+        for _ in g():
+            pass
+        raise
+
+
+def test_from_python():
+    """
+    >>> def test(g):
+    ...     try:
+    ...         1 / 0
+    ...     except:
+    ...         for _ in g():
+    ...             pass
+    ...         raise
+
+    >>> def pygen():
+    ...     yield 1
+    >>> test(pygen)  # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    ZeroDivisionError: ...division ...by zero
+
+    >>> test(cygen)  # doctest: +ELLIPSIS
+    Traceback (most recent call last):
+    ZeroDivisionError: ...division ...by zero
+    """
+
+
+def test_from_console():
+    """
+    >>> def pygen(): yield 1
+    >>> try:  # doctest: +ELLIPSIS
+    ...     1 / 0
+    ... except:
+    ...     for _ in pygen():
+    ...         pass
+    ...     raise
+    Traceback (most recent call last):
+    ZeroDivisionError: ...division ...by zero
+
+    >>> try:  # doctest: +ELLIPSIS
+    ...     1 / 0
+    ... except:
+    ...     for _ in cygen():
+    ...         pass
+    ...     raise
+    Traceback (most recent call last):
+    ZeroDivisionError: ...division ...by zero
+    """
diff --git a/tests/run/generators_py.py b/tests/run/generators_py.py
index 152e06a39..2396c8211 100644
--- a/tests/run/generators_py.py
+++ b/tests/run/generators_py.py
@@ -1,6 +1,7 @@
 # mode: run
 # tag: generators
 
+import sys
 import cython
 
 
@@ -147,25 +148,34 @@ def check_throw():
         except ValueError:
             pass
 
+
 def check_yield_in_except():
     """
-    >>> import sys
-    >>> orig_exc = sys.exc_info()[0]
-    >>> g = check_yield_in_except()
-    >>> next(g)
-    >>> next(g)
-    >>> orig_exc is sys.exc_info()[0] or sys.exc_info()[0]
+    >>> try:
+    ...     raise TypeError("RAISED !")
+    ... except TypeError as orig_exc:
+    ...     assert isinstance(orig_exc, TypeError), orig_exc
+    ...     g = check_yield_in_except()
+    ...     print(orig_exc is sys.exc_info()[1] or sys.exc_info())
+    ...     next(g)
+    ...     print(orig_exc is sys.exc_info()[1] or sys.exc_info())
+    ...     next(g)
+    ...     print(orig_exc is sys.exc_info()[1] or sys.exc_info())
+    True
+    True
     True
     """
     try:
         yield
         raise ValueError
-    except ValueError:
+    except ValueError as exc:
+        assert sys.exc_info()[1] is exc, sys.exc_info()
         yield
+        assert sys.exc_info()[1] is exc, sys.exc_info()
+
 
 def yield_in_except_throw_exc_type():
     """
-    >>> import sys
     >>> g = yield_in_except_throw_exc_type()
     >>> next(g)
     >>> g.throw(TypeError)
@@ -177,12 +187,14 @@ def yield_in_except_throw_exc_type():
     """
     try:
         raise ValueError
-    except ValueError:
+    except ValueError as exc:
+        assert sys.exc_info()[1] is exc, sys.exc_info()
         yield
+        assert sys.exc_info()[1] is exc, sys.exc_info()
+
 
 def yield_in_except_throw_instance():
     """
-    >>> import sys
     >>> g = yield_in_except_throw_instance()
     >>> next(g)
     >>> g.throw(TypeError())
@@ -194,8 +206,11 @@ def yield_in_except_throw_instance():
     """
     try:
         raise ValueError
-    except ValueError:
+    except ValueError as exc:
+        assert sys.exc_info()[1] is exc, sys.exc_info()
         yield
+        assert sys.exc_info()[1] is exc, sys.exc_info()
+
 
 def test_swap_assignment():
     """