Issue29587
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2017-02-17 11:01 by njs, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 19811 | merged | chris.jerdonek, 2020-04-30 08:48 | |
| PR 19821 | merged | vstinner, 2020-04-30 20:42 | |
| PR 19823 | merged | chris.jerdonek, 2020-04-30 21:43 | |
| PR 19859 | merged | chris.jerdonek, 2020-05-02 10:13 | |
| PR 19877 | merged | chris.jerdonek, 2020-05-03 04:28 | |
| PR 19902 | merged | vstinner, 2020-05-04 14:12 | |
| PR 19858 | merged | chris.jerdonek, 2020-05-06 10:27 | |
| Messages (30) | |||
|---|---|---|---|
| msg287987 - (view) | Author: Nathaniel Smith (njs) * ![]() |
Date: 2017-02-17 11:01 | |
Example 1:
-----------
def f():
try:
raise KeyError
except Exception:
yield
gen = f()
gen.send(None)
gen.throw(ValueError)
---------
Output:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<stdin>", line 5, in f
ValueError
Expected output:
Something involving the string "During handling of the above exception, another exception occurred", and a traceback for the original KeyError.
Example 2:
-----------
import sys
def f():
try:
raise KeyError
except Exception:
print(sys.exc_info())
try:
yield
except Exception:
pass
print(sys.exc_info())
raise
gen = f()
gen.send(None)
gen.throw(ValueError)
-----------
Output:
(<class 'KeyError'>, KeyError(), <traceback object at 0x7f67ce3c3f88>)
(None, None, None)
Traceback (most recent call last):
File "/tmp/foo.py", line 17, in <module>
gen.throw(ValueError)
File "/tmp/foo.py", line 13, in f
raise
RuntimeError: No active exception to reraise
Expected output: certainly not that :-)
This seems to happen because normally, generators save the current exc_info when yielding, and then restore it when re-entered. But, if we re-enter through 'throw' (throwflag is true), this is disabled:
https://github.com/python/cpython/blob/b2ee40ed9c9041dcff9c898aa19aacf9ec60308a/Python/ceval.c#L1027
This check seems to have been added in ae5f2f4a39e6a3f4c45e9dc95bd4e1fe5dfb60f2 as a fix for:
https://bugs.python.org/issue7173
which had to do with a nasty situation involving a generator object that was part of a reference cycle: the gc sometimes would free the objects stored in the generator's saved exc_info, and then try to clean up the generator by throwing in a GeneratorExit.
AFAICT this situation shouldn't be possible anymore thanks to PEP 442, which makes it so that finalizers are run before any part of the cycle is freed. And in any case it certainly doesn't justify breaking code like the examples above.
(Note: the examples use generators for simplicity, but of course the way I noticed this was that I had some async/await code where exceptions were mysteriously disappearing instead of showing up in __context__ and couldn't figure out why. It's likely that more people will run into this in the future as async/await becomes more widely used. As a workaround for now I'll probably modify my coroutine runner so that it never uses 'throw'.)
|
|||
| msg287992 - (view) | Author: Martin Panter (martin.panter) * ![]() |
Date: 2017-02-17 11:49 | |
The second example seems like the original complaint in Issue 25612. That spawned a separate bug related to the first situation, which was later closed: Issue 25683. |
|||
| msg306100 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2017-11-12 02:34 | |
Yury do you agree this is a bug? Do you have any ideas on how to fix it? |
|||
| msg306158 - (view) | Author: Yury Selivanov (yselivanov) * ![]() |
Date: 2017-11-13 15:00 | |
Guido,
The second example ("No active exception to reraise") was an actual long open bug. It was recently fixed by Mark Shannon in [1].
I think we should be able to fix the first case (missing __context__) although it's not critical. I'll look into it after the context PEP and few other open asyncio issues.
[1] https://github.com/python/cpython/pull/1773
|
|||
| msg367747 - (view) | Author: Chris Jerdonek (chris.jerdonek) * ![]() |
Date: 2020-04-30 09:20 | |
I made a naive attempt at addressing this issue here: https://github.com/python/cpython/pull/19811 The code has diverged significantly since Nathaniel first linked to a specific line above. However, what I came up with is simple, and seems to work. But I could very well be missing some things. It would be great if this could be fixed in some way. |
|||
| msg367773 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2020-04-30 19:18 | |
New changeset 2514a632fb7d37be24c2059d0e286d35600f9795 by Chris Jerdonek in branch 'master': bpo-29587: Enable implicit exception chaining with gen.throw() (GH-19811) https://github.com/python/cpython/commit/2514a632fb7d37be24c2059d0e286d35600f9795 |
|||
| msg367774 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2020-04-30 19:19 | |
Example 1 is now fixed too by Chris Jerdonek's PR 19811. Thanks Chris! |
|||
| msg367776 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2020-04-30 19:34 | |
Oh, no! Buildbot failures. Help?! |
|||
| msg367783 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-04-30 20:44 | |
New changeset 3c7f9db85095952821f9d106dd874f75662ce7ec by Victor Stinner in branch 'master': Revert "bpo-29587: Enable implicit exception chaining with gen.throw() (GH-19811)" (#19821) https://github.com/python/cpython/commit/3c7f9db85095952821f9d106dd874f75662ce7ec |
|||
| msg367784 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-04-30 20:47 | |
> bpo-29587: Enable implicit exception chaining with gen.throw() (GH-19811) > https://github.com/python/cpython/commit/2514a632fb7d37be24c2059d0e286d35600f9795 Sorry, I had to revert this change since it broke like 41+ buildbots: https://pythondev.readthedocs.io/ci.html#revert-on-fail Almost all CIs passed on the PR (except of the Ubuntu job of Azure Pipelines). That's unusual. No idea why the bug occurs only on some platforms and why it wasn't seen before. The revert is an opportunity to get more time to investigate the issue, without having the pressure to have to fix the CI "ASAP". |
|||
| msg367786 - (view) | Author: STINNER Victor (vstinner) * ![]() |
Date: 2020-04-30 21:02 | |
> Sorry, I had to revert this change since it broke like 41+ buildbots: (...) If someone wants to investigate, you can find examples of failed buildbot builds at: https://github.com/python/cpython/pull/19811 |
|||
| msg367787 - (view) | Author: Chris Jerdonek (chris.jerdonek) * ![]() |
Date: 2020-04-30 21:21 | |
> Oh, no! Buildbot failures. Help?! Yes, I'm sorry. There's something not so simple going on that I haven't yet reproduced locally. Will reply on the tracker. |
|||
| msg367788 - (view) | Author: Ned Deily (ned.deily) * ![]() |
Date: 2020-04-30 22:15 | |
Whatever the resolution of this is, it seems to me that this sort of behavior change should not be introduced at this stage of 3.7.x's life. Whether it should go into 3.8.x should be Łukasz's call once the final change is in master and has stabilized. |
|||
| msg367789 - (view) | Author: Yury Selivanov (yselivanov) * ![]() |
Date: 2020-04-30 22:24 | |
IMO this is a 3.9 fix. |
|||
| msg367792 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2020-04-30 22:38 | |
Could it be running out of memory due to excessive chaining of tracebacks? (And yes, it's 3.9 only.) |
|||
| msg367827 - (view) | Author: Chris Jerdonek (chris.jerdonek) * ![]() |
Date: 2020-05-01 08:56 | |
Okay, I was able to get the tests passing on the other platforms. I'm still not sure why Mac and Fedora behaved differently with my original PR. I was able to come up with a simple script that isolated the behavior difference (posted in the PR comments). It's very strange. Maybe it signals an issue elsewhere in the Python code base. |
|||
| msg367833 - (view) | Author: Chris Jerdonek (chris.jerdonek) * ![]() |
Date: 2020-05-01 10:03 | |
By the way, I just posted a new issue about exception chaining getting suppressed, but this time in an asyncio setting with ensure_future(): https://bugs.python.org/issue40466 I first noticed this issue two or three years ago and raised it on the async-sig list. It was suggested there that this was a special case of the current issue. Having written code to fix the current issue, though, the ensure_future() issue still exists. So I filed a separate issue. |
|||
| msg367905 - (view) | Author: Chris Jerdonek (chris.jerdonek) * ![]() |
Date: 2020-05-02 01:14 | |
New changeset 02047265eb83a43ba18cc7fee81756f1a1a1f968 by Chris Jerdonek in branch 'master': bpo-29587: Update gen.throw() to chain exceptions (#19823) https://github.com/python/cpython/commit/02047265eb83a43ba18cc7fee81756f1a1a1f968 |
|||
| msg367907 - (view) | Author: Chris Jerdonek (chris.jerdonek) * ![]() |
Date: 2020-05-02 01:26 | |
Okay, thanks everyone. I think I'll take a look at issue 40466 next. |
|||
| msg367908 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2020-05-02 01:37 | |
Wow, thanks all! FWIW I agree that ideally things should work with a NULL value... -- --Guido (mobile) |
|||
| msg367924 - (view) | Author: Chris Jerdonek (chris.jerdonek) * ![]() |
Date: 2020-05-02 10:28 | |
FYI, I just created a PR to add one more test to the prior fix, to test a slightly different aspect. Whereas the test in the first PR checks that exc.__context__ is set after gen.throw() is finished, the new test checks that exc.__context__ is available also from within the generator (while the generator is running). I'm adding this test partly because this behavior is different from the "awaiting on a task" case, which I'm working on next. Both of these tests were failing prior to the fix, which I confirmed. |
|||
| msg367926 - (view) | |||
