Repository navigation
nested try..excepts don't work correctly for generators #69798
Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2017-10-30.11:37:04.064>
created_at = <Date 2015-11-12.20:59:18.932>
labels = ['interpreter-core', '3.7']
title = "nested try..excepts don't work correctly for generators"
updated_at = <Date 2018-06-29.12:41:13.832>
user = 'https://github.com/1st1'bugs.python.org fields:
activity = <Date 2018-06-29.12:41:13.832>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2017-10-30.11:37:04.064>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2015-11-12.20:59:18.932>
creator = 'yselivanov'
dependencies = []
files = ['41026']
hgrepos = []
issue_num = 25612
keywords = ['patch']
message_count = 38.0
messages = ['254557', '254558', '254561', '254564', '254576', '254578', '254809', '254812', '254815', '254821', '254822', '254854', '254858', '254870', '254990', '254991', '254995', '254996', '255039', '255079', '256622', '287995', '287996', '304117', '304760', '304770', '304771', '305150', '305151', '305220', '305223', '317417', '317442', '319358', '319360', '319361', '319366', '320715']
nosy_count = 16.0
nosy_names = ['brett.cannon', 'ncoghlan', 'pitrou', 'scoder', 'vstinner', 'larry', 'benjamin.peterson', 'ned.deily', 'njs', 'asvetlov', 'chris.jerdonek', 'Mark.Shannon', 'martin.panter', 'serhiy.storchaka', 'yselivanov', 'emptysquare']
pr_nums = ['1773', '7069', '7074', '7656', '7658']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue25612'
versions = ['Python 3.7']
Activity
1st1 commentedon Nov 12, 2015
Nested try..except statements with yields can loose reference to the current exception.
The following code:
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 reraise1st1 commentedon Nov 12, 2015
This was originally discovered here: python/asyncio#287
vstinner commentedon Nov 12, 2015
Is this issue related to the issue bpo-23353?
vstinner commentedon Nov 12, 2015
I reverted the change of the issue bpo-23353 but it doesn't fix this example, so it looks like these issues are not related. Cool.
1st1 commentedon Nov 12, 2015
Another bug:
SubError will propagate, but won't have MainError in its __context__
1st1 commentedon Nov 13, 2015
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)
1st1 commentedon Nov 17, 2015
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).
gvanrossum commentedon Nov 17, 2015
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.
vadmium commentedon Nov 17, 2015
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.
vstinner commentedon Nov 17, 2015
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.
vadmium commentedon Nov 17, 2015
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.
1st1 commentedon Nov 18, 2015
Not sure I understand what you're saying here.
vadmium commentedon Nov 18, 2015
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.
# 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.
16 remaining items
pitrou commentedon Oct 22, 2017
Thank you Mark for the fix!
serhiy-storchaka commentedon Oct 28, 2017
Note that removing exc_type, exc_value and exc_traceback from PyThreadState breaks Cython.
serhiy-storchaka commentedon Oct 28, 2017
The problem I mentioned in msg304117 has been resolved in backward direction: "raise" outside of an except block don't raise a RuntimeError.
markshannon commentedon Oct 30, 2017
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, butraisejust 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.
Is there a matching Cython issue?
serhiy-storchaka commentedon Oct 30, 2017
Thank you for the clarification Mark. I agree that the current behavior is well justified.
Cython was fixed in cython/cython@2d33924.
vstinner commentedon May 23, 2018
New changeset b6dccf5 by Victor Stinner in branch 'master':
bpo-33612: Remove PyThreadState_Clear() assertion (bpo-7069)
b6dccf5
ned-deily commentedon May 23, 2018
New changeset 508d769 by Ned Deily (Miss Islington (bot)) in branch '3.7':
bpo-33612: Remove PyThreadState_Clear() assertion (GH-7069) (GH-7074)
508d769
ned-deily commentedon Jun 12, 2018
New changeset 04290cb by Ned Deily in branch 'master':
bpo-25612: Add minimal What's New in 3.7 entry (GH-7656)
04290cb
ned-deily commentedon Jun 12, 2018
New changeset ef5f4ba 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)
ef5f4ba
ned-deily commentedon Jun 12, 2018
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.
serhiy-storchaka commentedon Jun 12, 2018
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.
vstinner commentedon Jun 29, 2018
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".