| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Before this commit, if an exception was active inside a generator when calling gen.throw(), then that exception was lost (i.e. there was no implicit exception chaining). This commit fixes that.
There was a problem hiding this comment.
test_unittest still crash on this PR. I tested on Linux (Fedora 32).
$ make clean $ make $ ./python -m test -v test_unittest (...) test_patch_nested_autospec_repr (unittest.test.testmock.testpatch.PatchTest) ... ok test_patch_object_keyword_args (unittest.test.testmock.testpatch.PatchTest) ... ok test_patch_object_with_spec_as_boolean (unittest.test.testmock.testpatch.PatchTest) ... ok test_patch_orderdict (unittest.test.testmock.testpatch.PatchTest) ... ok test_patch_propagates_exc_on_exit (unittest.test.testmock.testpatch.PatchTest) ... Fatal Python error: Segmentation fault Current thread 0x00007f886bd1c740 (most recent call first): File "/home/vstinner/python/master/Lib/unittest/mock.py", line 1524 in __exit__ File "/home/vstinner/python/master/Lib/unittest/test/testmock/testpatch.py", line 1673 in __exit__ File "/home/vstinner/python/master/Lib/contextlib.py", line 498 in __exit__ File "/home/vstinner/python/master/Lib/unittest/mock.py", line 1323 in decoration_helper File "/home/vstinner/python/master/Lib/contextlib.py", line 135 in __exit__ File "/home/vstinner/python/master/Lib/unittest/mock.py", line 1337 in patched File "/home/vstinner/python/master/Lib/unittest/case.py", line 201 in handle File "/home/vstinner/python/master/Lib/unittest/case.py", line 733 in assertRaises File "/home/vstinner/python/master/Lib/unittest/test/testmock/testpatch.py", line 1692 in test_patch_propagates_exc_on_exit (...)
Sorry, something went wrong.
|
When you're done making the requested changes, leave the comment: I have made the requested changes; please review again. |
Sorry, something went wrong.
|
Is there a way to mark this PR as "don't merge" for now? This is the same PR as before, so it has the same issue. The Azure CI job seems not to be running, which is what exhibited the failure before.
Yes, this is the same failure as before. Everything passes locally on my Mac, so it will take me more time to look into this. |
Sorry, something went wrong.
Hum, I wasn't sure, so I did the 3 tricks that I know:
It seems like only the conversion to a draft technically prevents someone to merge the PR my mistake. |
Sorry, something went wrong.
|
Looks like that will cover it. ;) |
Sorry, something went wrong.
|
Okay, I believe my latest changes fix the buildbot failures from before. Also, for future reference, below is a script I came up with that isolates the behavior difference between Mac and Fedora, with my prior version of the PR: import re
import sys
def f():
exc, exc_value, tb = sys.exc_info()
print(f'CLEARING FRAME: {tb.tb_frame!r}')
tb.tb_frame.clear()
def g():
# Uncommenting the following line caused the tb_frame.clear() line
# above to exhibit the following platform-specific behavior:
# 1) On Mac, this is logged to stderr
# > TypeError: print_exception(): Exception expected for value,
# NoneType found
# 2) On Fedora 32, the following error happens:
# > Fatal Python error: Segmentation fault
data = re.compile('xxx')
try:
yield
except Exception:
f()
gen = g()
gen.send(None)
gen.throw(ValueError)Maybe this suggests an issue elsewhere in Python's code base. Should this PR also get a What's New? |
Sorry, something went wrong.
|
I have made the requested changes; please review again. |
Sorry, something went wrong.
|
Thanks for making the requested changes! @vstinner: please review the changes made to this pull request. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM. "./python -m test -j0 -r" does no longer crash on my Fedora.
I understood that passing NULL value to _PyErr_ChainExceptions() created the bug? Maybe add an assertion to prevent the situation to happen again?
Sorry, something went wrong.
|
Thank you. Regarding an assertion, the thing is that it looks like _PyErr_ChainExceptions() is designed to accept a NULL value. So maybe in the future it can be made to work. (That's why I added an XXX comment.) So I wouldn't want to add an assertion making it seem like it always has to be like that. It could just something about how things are set up right now that was causing the crash. Would you still like me to add an assertion and/or perhaps a code comment explaining this possibility? |
Sorry, something went wrong.
|
@cjerdonek: Please replace # with GH- in the commit message next time. Thanks! |
Sorry, something went wrong.
This is a follow-up to GH-19823 that removes the check that the exception value isn't NULL, prior to calling _PyErr_ChainExceptions(). This enables implicit exception chaining for gen.throw() in more circumstances. The commit also adds a test that a particular code snippet involving gen.throw() doesn't crash. The test shows why the new `gi_exc_state.exc_type != Py_None` check that was added is necessary. Without the new check, the code snippet (as well as a number of other tests) crashes on certain platforms (e.g. Fedora but not Mac).
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This is a new version of PR #19811 to sort out the buildbot failures.
It enables implicit exception chaining when calling generator.throw(exc) by setting exc.__context__.
https://bugs.python.org/issue29587