bpo-24596: Decref module in PyRun_SimpleFileExFlags() on SystemExit by ZackerySpytz · Pull Request #7918 · python/cpython (original) (raw)
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation10 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
PyErr_Print() will not return when the exception is a SystemExit, so decref the main module object in that case.
| @@ -431,6 +431,9 @@ PyRun_SimpleFileExFlags(FILE *fp, const char *filename, int closeit, |
|---|
| } |
| flush_io(); |
| if (v == NULL) { |
| if (PyErr_ExceptionMatches(PyExc_SystemExit)) { |
| Py_DECREF(m); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why make this a special case? We could just always Py_DECREF(m) early.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. Py_DECREF(m) would then be called one too many times.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor refactoring would be needed, but I don't think that's insurmountable (perhaps you can use Py_CLEAR instead).
| @@ -453,6 +453,24 @@ def test_del___main__(self): |
|---|
| print("del sys.modules['__main__']", file=script) |
| assert_python_ok(filename) |
| def test_global_del_SystemExit(self): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think test_cmd_line is the right place for this. test_gc already has related tests (see e.g. test_gc_main_module_at_shutdown() there).
@pitrou Thanks for the comments. I've updated the PR.
Thanks @ZackerySpytz for the PR, and @pitrou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖
Thanks @ZackerySpytz for the PR, and @pitrou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
PyErr_Print() will not return when the exception is a SystemExit, so decref the main module object in that case. (cherry picked from commit d8cba5d)
Co-authored-by: Zackery Spytz zspytz@gmail.com
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request
PyErr_Print() will not return when the exception is a SystemExit, so decref the main module object in that case. (cherry picked from commit d8cba5d)
Co-authored-by: Zackery Spytz zspytz@gmail.com
pitrou pushed a commit that referenced this pull request
PyErr_Print() will not return when the exception is a SystemExit, so decref the main module object in that case. (cherry picked from commit d8cba5d)
Co-authored-by: Zackery Spytz zspytz@gmail.com
pitrou pushed a commit that referenced this pull request
PyErr_Print() will not return when the exception is a SystemExit, so decref the main module object in that case. (cherry picked from commit d8cba5d)
Co-authored-by: Zackery Spytz zspytz@gmail.com