homepage

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.

classification
Title: Generator/coroutine 'throw' discards exc_info state, which is bad
Type: behavior Stage: resolved
Components: Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: chris.jerdonek, gvanrossum, hynek, martin.panter, ned.deily, njs, vstinner, yselivanov
Priority: normal Keywords: patch

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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2020年04月30日 19:34
Oh, no! Buildbot failures. Help?!
msg367783 - (view) Author: STINNER Victor (vstinner) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2020年04月30日 22:24
IMO this is a 3.9 fix.
msg367792 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020年05月02日 11:48
FYI, I was able to finish addressing the other exception-chaining cases (the ones covered by the other issue) with a simple change to the fix for this PR.
I moved the call to _PyErr_ChainExceptions() a bit deeper: into gen_send_ex() before the call to _PyEval_EvalFrame(), and behind an `if (exc)` check.
So I think the tracebacks should improve a lot.
msg367954 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020年05月03日 04:57
Okay, I was able to remove the NULL value check and get things working with a NULL exception value. I just posted a second follow-on PR that does this (which Guido approved - thanks, Guido!):
https://github.com/python/cpython/pull/19877
I wanted to tell you what I found since it raises some questions.
To remove that check I had to add the following new check prior to calling `_PyErr_ChainExceptions()` with the exception info, as a replacement:
`gen->gi_exc_state.exc_type != Py_None`
Without doing this, code like the following would crash e.g. on Fedora 32 (this is the crash that was happening in the first version of my PR, reduced down):
 def g():
 try:
 raise KeyError
 except KeyError:
 pass
 try:
 yield
 except Exception:
 # Crash happens here e.g. on Fedora 32 but not Mac.
 raise RuntimeError
 gen = g()
 gen.send(None)
 gen.throw(ValueError)
This raises two questions for me:
First, I thought exc_type could only ever be NULL or an exception class. So I'm wondering if this points to a problem elsewhere in the code base.
Second, I don't know why it would crash on Fedora but not Mac. (On Mac, you instead see the following exception chained beneath the ValueError:
> TypeError: print_exception(): Exception expected for value, NoneType found )
msg367956 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020年05月03日 07:08
New changeset 21893fbb74e8fde2931fbed9b511e2a41362b1ab by Chris Jerdonek in branch 'master':
bpo-29587: allow chaining NULL exceptions in _gen_throw() (GH-19877)
https://github.com/python/cpython/commit/21893fbb74e8fde2931fbed9b511e2a41362b1ab
msg368013 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020年05月04日 07:50
Since none of you are subscribed to the other issue except for Nathaniel, I thought I'd let you know I also posted a PR to fix another issue he filed similar to this one (but this time about the stack trace being incorrect for `gen.throw()` in certain circumstances): https://bugs.python.org/issue29590
That issue is the second of the two issues he cited back in 2017 about `gen.throw()` being broken:
https://vorpus.org/~njs/misc/trio-language-summit-2017.pdf 
msg368053 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020年05月04日 14:13
tl; dr I wrote PR 19902 to remove the "XXX" comment.
--
Commit 21893fbb74e8fde2931fbed9b511e2a41362b1ab adds the following code:
 /* XXX It seems like we shouldn't have to check not equal to Py_None
 here because exc_type should only ever be a class. But not including
 this check was causing crashes on certain tests e.g. on Fedora. */
 if (gen->gi_exc_state.exc_type && gen->gi_exc_state.exc_type != Py_None) { ... }
I don't think that you should mention "Fedora" as a platform impacted by this issue: all platforms should be affected. It's just unclear why the issue was first seen on Fedora.
gen_send_ex() copies tstate->exc_info to gen->gi_exc_state.
tstate->exc_info->exc_type is set at least in the following 3 places in CPython code base:
* ceval.c: POP_EXCEPT opcode: exc_info->exc_type = POP();
* ceval.c: UNWIND_EXCEPT_HANDLER() macro: exc_info->exc_type = POP();
* errors.c: PyErr_SetExcInfo()
I saw in practice POP_EXCEPT and UNWIND_EXCEPT_HANDLER() setting exc_info->exc_type to Py_None (I didn't test PyErr_SetExcInfo()).
_PyErr_GetTopmostException() also handles exc_info->exc_type == Py_None case:
_PyErr_StackItem *
_PyErr_GetTopmostException(PyThreadState *tstate)
{
 _PyErr_StackItem *exc_info = tstate->exc_info;
 while ((exc_info->exc_type == NULL || exc_info->exc_type == Py_None) &&
 exc_info->previous_item != NULL)
 {
 exc_info = exc_info->previous_item;
 }
 return exc_info;
}
--
So exc_type=None is not a bug, but it's done on purpose.
If you don't want to get None in genobject.c, we should modify all places which set tstate->exc_info->exc_type. Problem: the structure currently exposed in the public C API (bpo-40429), and I wouldn't be surprised if Cython or greenlet modify tstate->exc_info directly.
msg368164 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020年05月05日 15:07
New changeset b0be6b3b94fbdf31b796adc19dc86a04a52b03e1 by Victor Stinner in branch 'master':
bpo-29587: _PyErr_ChainExceptions() checks exception (GH-19902)
https://github.com/python/cpython/commit/b0be6b3b94fbdf31b796adc19dc86a04a52b03e1
msg368617 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020年05月11日 07:53
Would someone be able to approve / take a look at this small PR addressing another aspect of this issue?
https://github.com/python/cpython/pull/19858
I'm hoping it can be part of 3.9, and the deadline is just a week away.
It builds on the already merged PR to address an "Example 3" of this issue (the "yield from" case as opposed to the "yield" case, which the merged PR addressed):
Example 3:
def f():
 yield
def g():
 try:
 raise KeyError('a')
 except Exception:
 yield from f()
gen = g()
gen.send(None)
gen.throw(ValueError)
----------------------
Output without PR:
Traceback (most recent call last):
 File "/.../test-example3.py", line 12, in <module>
 gen.throw(ValueError)
 File "/.../test-example3.py", line 8, in g
 yield from f()
 File "/.../test-example3.py", line 2, in f
 yield
ValueError
----------------------
Output with PR:
Traceback (most recent call last):
 File "/.../test-example3.py", line 6, in g
 raise KeyError('a')
KeyError: 'a'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
 File "/.../test-example3.py", line 12, in <module>
 gen.throw(ValueError)
 File "/.../test-example3.py", line 8, in g
 yield from f()
 File "/.../test-example3.py", line 2, in f
 yield
ValueError
msg368806 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020年05月13日 23:18
New changeset 75cd8e48c62c97fdb9d9a94fd2335be06084471d by Chris Jerdonek in branch 'master':
bpo-29587: Make gen.throw() chain exceptions with yield from (GH-19858)
https://github.com/python/cpython/commit/75cd8e48c62c97fdb9d9a94fd2335be06084471d
msg369093 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020年05月17日 04:14
New changeset d7184d3dbd249444ec3961641dc08a9ad3c1ac34 by Chris Jerdonek in branch 'master':
bpo-29587: Add another test for the gen.throw() fix. (GH-19859)
https://github.com/python/cpython/commit/d7184d3dbd249444ec3961641dc08a9ad3c1ac34
History
Date User Action Args
2022年04月11日 14:58:43adminsetgithub: 73773
2020年05月17日 04:14:59chris.jerdoneksetmessages: + msg369093
2020年05月13日 23:18:30chris.jerdoneksetmessages: + msg368806
2020年05月11日 07:53:14chris.jerdoneksetmessages: + msg368617
2020年05月06日 10:27:00chris.jerdoneksetpull_requests: + pull_request19266
2020年05月05日 15:07:49vstinnersetmessages: + msg368164
2020年05月04日 14:13:00vstinnersetmessages: + msg368053
2020年05月04日 14:12:14vstinnersetpull_requests: + pull_request19216
2020年05月04日 07:50:07chris.jerdoneksetmessages: + msg368013
2020年05月03日 07:08:10chris.jerdoneksetmessages: + msg367956
2020年05月03日 04:57:23chris.jerdoneksetmessages: + msg367954
2020年05月03日 04:28:22chris.jerdoneksetpull_requests: + pull_request19189
2020年05月02日 11:48:24chris.jerdoneksetmessages: + msg367926
2020年05月02日 10:28:27chris.jerdoneksetmessages: + msg367924
2020年05月02日 10:13:06chris.jerdoneksetpull_requests: + pull_request19175
2020年05月02日 01:37:54gvanrossumsetmessages: + msg367908
2020年05月02日 01:26:31chris.jerdoneksetstatus: open -> closed
resolution: fixed
messages: + msg367907

stage: patch review -> resolved
2020年05月02日 01:14:25chris.jerdoneksetmessages: + msg367905
2020年05月01日 10:03:18chris.jerdoneksetmessages: + msg367833
2020年05月01日 08:56:38chris.jerdoneksetmessages: + msg367827
2020年04月30日 22:38:15gvanrossumsetmessages: + msg367792
2020年04月30日 22:24:59yselivanovsetmessages: + msg367789
2020年04月30日 22:15:09ned.deilysetnosy: + ned.deily

messages: + msg367788
versions: - Python 3.5, Python 3.6, Python 3.7, Python 3.8
2020年04月30日 21:43:47chris.jerdoneksetstage: resolved -> patch review
pull_requests: + pull_request19143
2020年04月30日 21:21:46chris.jerdoneksetmessages: + msg367787
2020年04月30日 21:02:46vstinnersetmessages: + msg367786
2020年04月30日 20:47:40vstinnersetmessages: + msg367784
2020年04月30日 20:46:52gvanrossumsetstatus: closed -> open
resolution: fixed -> (no value)
2020年04月30日 20:44:27vstinnersetmessages: + msg367783
2020年04月30日 20:42:18vstinnersetnosy: + vstinner

pull_requests: + pull_request19141
2020年04月30日 19:34:16gvanrossumsetmessages: + msg367776
2020年04月30日 19:19:18gvanrossumsetstatus: open -> closed
resolution: fixed
messages: + msg367774

stage: patch review -> resolved
2020年04月30日 19:18:13gvanrossumsetmessages: + msg367773
2020年04月30日 09:20:36chris.jerdoneksetmessages: + msg367747
versions: + Python 3.8, Python 3.9
2020年04月30日 08:48:51chris.jerdoneksetkeywords: + patch
stage: patch review
pull_requests: + pull_request19131
2017年11月13日 15:00:45yselivanovsetmessages: + msg306158
2017年11月12日 02:34:25gvanrossumsetnosy: + gvanrossum, yselivanov
messages: + msg306100
2017年11月11日 09:54:31hyneksetnosy: + hynek
2017年11月11日 09:02:15chris.jerdoneksetnosy: + chris.jerdonek
2017年02月17日 11:49:20martin.pantersettype: behavior

messages: + msg287992
nosy: + martin.panter
2017年02月17日 11:01:49njscreate

AltStyle によって変換されたページ (->オリジナル) /