| msg254557 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2015-11-12 20:59 |
| 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 coro.throw(SubError()) File "t.py", line 15, in main raise RuntimeError: No active exception to reraise |
|
|
| msg254558 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2015-11-12 21:13 |
| This was originally discovered here: https://github.com/python/asyncio/issues/287 |
|
|
| msg254561 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2015-11-12 22:14 |
| Is this issue related to the issue #23353? |
|
|
| msg254564 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2015-11-12 22:18 |
| 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. |
|
|
| msg254576 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2015-11-12 23:42 |
| 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__ |
|
|
| msg254578 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2015-11-13 00:16 |
| 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) |
|
|
| msg254809 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2015-11-17 17:59 |
| 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). |
|
|
| msg254812 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2015-11-17 18:52 |
| 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. |
|
|
| msg254815 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2015-11-17 21:36 |
| 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. |
|
|
| msg254821 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2015-11-17 23:27 |
| > 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. |
|
|
| msg254822 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2015-11-17 23:36 |
| 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. |
|
|
| msg254854 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2015-11-18 19:31 |
| > 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. |
|
|
| msg254858 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2015-11-18 22:22 |
| 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. |
|
|
| msg254870 - (view) |
Author: Larry Hastings (larry) * |
Date: 2015-11-19 01:18 |
| I don't plan to hold up 3.5.1 for this. |
|
|
| msg254990 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2015-11-20 16:14 |
| 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 |
|
|
| msg254991 - (view) |
Author: Guido van Rossum (gvanrossum) * |
Date: 2015-11-20 16:16 |
| 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> > _______________________________________ |
|
|
| msg254995 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2015-11-20 16:30 |
| Anyways, here's a separate issue for the __context__ problem: http://bugs.python.org/issue25683 |
|
|
| msg254996 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2015-11-20 16:31 |
| Guido, Martin, I've just posted to python-dev about this ticket. Let's see what others think. |
|
|
| msg255039 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2015-11-21 03:28 |
| 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. |
|
|
| msg255079 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2015-11-22 00:56 |
| Nick, your scenario seems to behave no differently with and without the patch: Traceback (most recent call last): File "", line 2, in __main__.MainError |
|
|
| msg256622 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2015-12-17 23:09 |
| 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. |
|
|
| msg287995 - (view) |
Author: Nathaniel Smith (njs) * |
Date: 2017-02-17 12:01 |
| 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... |
|
|
| msg287996 - (view) |
Author: Nathaniel Smith (njs) * |
Date: 2017-02-17 12:05 |
| (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.) |
|
|
| msg304117 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-11 08:31 |
| 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 "", line 5, in File "", line 2, in ZeroDivisionError: division by zero With PR 1773 applied it raises: Traceback (most recent call last): File "", line 2, in ZeroDivisionError: division by zero During handling of the above exception, another exception occurred: Traceback (most recent call last): File "", line 5, in File "", 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? |
|
|
| msg304760 - (view) |
Author: Mark Shannon (Mark.Shannon) * |
Date: 2017-10-22 19:32 |
| Thanks Serhiy for spotting that. 'raise' should raise the same exception as sys.exc_info() returns. I'll update the PR. |
|
|
| msg304770 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2017-10-22 21:41 |
| 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 |
|
|
| msg304771 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2017-10-22 21:42 |
| Thank you Mark for the fix! |
|
|
| msg305150 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-28 08:16 |
| Note that removing exc_type, exc_value and exc_traceback from PyThreadState breaks Cython. |
|
|
| msg305151 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-28 08:22 |
| The problem I mentioned in has been resolved in backward direction: "raise" outside of an except block don't raise a RuntimeError. |
|
|
| msg305220 - (view) |
Author: Mark Shannon (Mark.Shannon) * |
Date: 2017-10-30 10:59 |
| 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? |
|
|
| msg305223 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-10-30 11:37 |
| 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. |
|
|
| msg317417 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-05-23 16:00 |
| New changeset b6dccf54fd3bac9c87348d96f9d6b571608c15bc by Victor Stinner in branch 'master': bpo-33612: Remove PyThreadState_Clear() assertion (#7069) https://github.com/python/cpython/commit/b6dccf54fd3bac9c87348d96f9d6b571608c15bc |
|
|
| msg317442 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2018-05-23 19:58 |
| 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 |
|
|
| msg319358 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2018-06-12 03:20 |
| 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 |
|
|
| msg319360 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2018-06-12 04:26 |
| 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 |
|
|
| msg319361 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2018-06-12 04:29 |
| 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. |
|
|
| msg319366 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2018-06-12 05:49 |
| 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. |
|
|
| msg320715 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2018-06-29 12:41 |
| 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". |
|
|