Nested try..except statements with yields can loose reference to the current exception.

The following code:

    class MainError(Exception):
        pass

    class SubError(Exception):
        pass

    def main():
        try:
            raise MainError()
        except MainError:
            try:
                yield
            except SubError:
                print('got SubError')
            raise

    coro = main()
    coro.send(None)
    coro.throw(SubError())

prints:

    got SubError
    Traceback (most recent call last):
      File "t.py", line 19, in <module>
        coro.throw(SubError())
      File "t.py", line 15, in main
        raise
    RuntimeError: No active exception to reraise

This was originally discovered here: https://github.com/python/asyncio/issues/287

Is this issue related to the issue #23353?

I reverted the change of the issue #23353 but it doesn't fix this example, so it looks like these issues are not related. Cool.

Another bug:

    class MainError(Exception):
        pass

    class SubError(Exception):
        pass

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

    coro = main()
    coro.send(None)
    coro.throw(SubError())


SubError will propagate, but won't have MainError in its __context__

Here's a patch the fixes the first problem (but __context__ bug is still open).

I'm not sure that the patch is correct :(  But at least I've added new unittests (one still failing)

Can someone work with me on fixing this issue?  I think it's important to ship 3.5.1 with this resolved (and 3.4.4 FWIW).

You might have to ping python-dev. But in terms of priorities I think
it's not a blocker -- it's been broken for quite some time now, and
it's a fairly odd corner of the language.

Regarding the second bug, did you consider that the exception thrown to the generator can already have __context__ set?

try:
    try: raise ValueError("Context outside of generator")
    except ValueError as ex: raise SubError() from ex
except SubError as ex:
    coro.throw(ex)  # ex.__context__ is a ValueError

I guess either one context could trump the other, or we could to follow the chain of contexts and append the other chain at the end. Both these ideas seem a bit ugly though.

> Can someone work with me on fixing this issue?  I think it's important to ship 3.5.1 with this resolved (and 3.4.4 FWIW).

It don't consider this issue as a blocker for Python 3.5.1. This
release already contains a *lot* of bugfixes! It's important to get it
as soon as possible.

Thinking about the __context__ thing some more, I guess it might make sense for __context__ to be overwritten by the generator context. The same way it gets overwritten when you execute “raise” inside an exception handler.

> Thinking about the __context__ thing some more, I guess it might make sense for __context__ to be overwritten by the generator context. The same way it gets overwritten when you execute “raise” inside an exception handler.

Not sure I understand what you're saying here.

I was making an analogy between how the “raise” statement works, and how the throw() method could work. In this example, there are three exception objects (MainError, SubError, and ValueError). I was suggesting that it is okay for the context to be set to the MainError instance, because that is how the analogous version using a “raise” statement works.

def main():
    try:
        raise MainError("Context inside generator")
    except MainError:
        yield  # __context__ could be changed to MainError

coro = main()
coro.send(None)
try:
    try: raise ValueError("Context outside of generator")
    except ValueError: raise SubError()
except SubError as ex:
    coro.throw(ex)  # __context__ is ValueError

# raise analogy:

try:
    try: raise ValueError("Context outside of generator")
    except ValueError as ex: raise SubError()
except SubError as ex:
    saved = ex  # __context__ is ValueError

try:
    raise MainError("Context inside generator")
except MainError:
    raise saved  # Changes __context__ to MainError

===

Sorry I’m not really familiar with the code to quickly propose a patch or review your change though.

I don't plan to hold up 3.5.1 for this.

Martin, you might be right here.  Guido, do you think it's OK that SubError doesn't have MainError in its __context__? http://bugs.python.org/issue25612#msg254576

I'm sorry, I'm no help here -- I don't know how __context__ is
supposed to work. :-(

On Fri, Nov 20, 2015 at 8:14 AM, Yury Selivanov <report@bugs.python.org> wrote:
>
> Yury Selivanov added the comment:
>
> Martin, you might be right here.  Guido, do you think it's OK that SubError doesn't have MainError in its __context__? http://bugs.python.org/issue25612#msg254576
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue25612>
> _______________________________________

Anyways, here's a separate issue for the __context__ problem: http://bugs.python.org/issue25683

Guido, Martin, I've just posted to python-dev about this ticket.  Let's see what others think.

Interrogating the thread state like that makes me wonder how this patch behaves in the following scenario:

    class MainError(Exception): pass
    class SubError(Exception): pass

    def yield_coro():
        yield
    coro = yield_coro()
    coro.send(None)

    try:
        raise MainError
    except MainError:
        try:
            coro.throw(SubError)
        except SubError:
            pass
        raise

Also potentially worth exploring is this proposed architectural change from Mark Shannon to move all exception related state out of frame objects: http://bugs.python.org/issue13897

While we couldn't do the latter in a maintenance release, I'd be interested to know what effect it has on this edge case.

Nick, your scenario seems to behave no differently with and without the patch:

Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
__main__.MainError

I closed issue #25683 as "not a bug".  So it's just this issue that we need to fix.

Anyone wants to review the patch? :)  Since the code involved here is quite complex, I don't want to commit this patch without a thorough review.

I read the patch but I don't understand the logic behind it :-). Why should the value of tstate->exc_type affect whether we save/restore exc_info? Won't this mean that things are still broken for code like:

-----
def f():
    try:
        raise KeyError
    except Exception:
        try:
            yield
        except Exception:
            pass
        raise

try:
    raise NameError
except Exception:
    gen = f()
    gen.send(None)
    gen.throw(ValueError)
-----

?

Conceptually I would have thought the fix would be to remove the '!throwflag' check, but I haven't actually tried it...

(Issue 29587 is a duplicate of this one, but it has some more information on where the !throwflag check came from and why it might be possible to remove it now.)

I tried the following code:

def g():
    yield 1
    raise
    yield 2

i = g()
try:
    1/0
except:
    next(i)
    next(i)

Currently it raises:

Traceback (most recent call last):
  File "<stdin>", line 5, in <module>
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero

With PR 1773 applied it raises:

Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
ZeroDivisionError: division by zero

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 5, in <module>
  File "<stdin>", line 3, in g
RuntimeError: No active exception to reraise

And this looks more correct.

But if replace raise with print(sys.exc_info()) the patched and unpatched interpreters both print:

(<class 'ZeroDivisionError'>, ZeroDivisionError('division by zero',), <traceback object at 0x7f61d9ed1448>)

Is it correct that sys.exc_info() return an exception while raise can't reraise it?

Thanks Serhiy for spotting that.

'raise' should raise the same exception as sys.exc_info() returns.
I'll update the PR.

New changeset ae3087c6382011c47db82fea4d05f8bbf514265d by Antoine Pitrou (Mark Shannon) in branch 'master':
Move exc state to generator. Fixes bpo-25612 (#1773)
https://github.com/python/cpython/commit/ae3087c6382011c47db82fea4d05f8bbf514265d

Thank you Mark for the fix!

Note that removing exc_type, exc_value and exc_traceback from PyThreadState breaks Cython.

The problem I mentioned in msg304117 has been resolved in backward direction: "raise" outside of an except block don't raise a RuntimeError.

Looking at the docs:

https://docs.python.org/3.6/library/sys.html#sys.exc_info states:
     If the current stack frame is not handling an exception, the information is taken from the calling stack frame, or its caller, and so on until a stack frame is found that is handling an exception.

And https://docs.python.org/3/reference/simple_stmts.html#the-raise-statement

If no expressions are present, raise re-raises the last exception that was active in the current scope. If no exception is active in the current scope, a RuntimeError exception is raised indicating that this is an error.

Note that `sys.exc_info()` explicitly mentions scanning the stack, but `raise` just says "active in the current scope". Testing on 3.5 shows that "active in the current scope" does scan the stack (for simple calls at least).

Which means that the newly implemented behaviour is correct.



> Note that removing exc_type, exc_value and exc_traceback from PyThreadState breaks Cython.

Is there a matching Cython issue?

Thank you for the clarification Mark. I agree that the current behavior is well justified.

Cython was fixed in https://github.com/cython/cython/commit/2d3392463b77eb550bd54a69bd28cc161947acb5.

New changeset b6dccf54fd3bac9c87348d96f9d6b571608c15bc by Victor Stinner in branch 'master':
bpo-33612: Remove PyThreadState_Clear() assertion (#7069)
https://github.com/python/cpython/commit/b6dccf54fd3bac9c87348d96f9d6b571608c15bc

New changeset 508d7693bc09affd99fdaa4a321cc3da0638c8a0 by Ned Deily (Miss Islington (bot)) in branch '3.7':
bpo-33612: Remove PyThreadState_Clear() assertion (GH-7069) (GH-7074)
https://github.com/python/cpython/commit/508d7693bc09affd99fdaa4a321cc3da0638c8a0

New changeset 04290cb9945eca1a97f6924495256c15f29fab41 by Ned Deily in branch 'master':
bpo-25612: Add minimal What's New in 3.7 entry (GH-7656)
https://github.com/python/cpython/commit/04290cb9945eca1a97f6924495256c15f29fab41

New changeset ef5f4ba96e05f61ad3baca502012085f31e99342 by Ned Deily (Miss Islington (bot)) in branch '3.7':
bpo-25612: Add minimal What's New in 3.7 entry (GH-7656) (GH-7658)
https://github.com/python/cpython/commit/ef5f4ba96e05f61ad3baca502012085f31e99342

P.S. I added a very minimal What's New in 3.7 entry for this change since it has affected some downline projects.  I just copied the NEWS entry.  Feel free to expand it and/or move it to a better location in the file.

This change was in top5 breaking changes for 3.7. It broke many projects that use Cython until they upgraded Cython to the version that supports 3.7.

It seems like this issue introduced a regression in Python 3.7.0: bpo-33996 "Crash in gen_send_ex(): _PyErr_GetTopmostException() returns freed memory".