gh-152132: Handle sys.exit() in Py_RunMain command and file paths#152191
Open
tangyuan0821 wants to merge 6 commits into
Open
gh-152132: Handle sys.exit() in Py_RunMain command and file paths#152191tangyuan0821 wants to merge 6 commits into
tangyuan0821 wants to merge 6 commits into
Conversation
Two issues introduced by the Py_RunMain SystemExit fix: 1. pymain_run_command lost traceback source lines for -c commands because PyRun_StringFlags does not register source in linecache. Add _PyRun_SimpleStringFlagsEx, which returns PyObject* without calling PyErr_Print(), and use it in pymain_run_command with the "<string>" name so source linecache registration is preserved. 2. _PyRun_SimpleFileObjectEx could lose exceptions during cleanup when PyDict_PopString fails (e.g. under low-memory conditions). Save and restore the original exception around cleanup code when the main code failed; use PyErr_Print() for cleanup errors when the main code succeeded to match legacy behavior.
ZeroIntensity
left a comment
ZeroIntensity
left a comment
Member
There was a problem hiding this comment.
Please add a test case.
- Rename _PyRun_SimpleFileObjectEx → _PyRun_SimpleFileObjectNoPrint to avoid the deprecated 'Ex' suffix. Make _PyRun_SimpleFileObject a thin wrapper around it to avoid code duplication. - Rename _PyRun_SimpleStringFlagsEx → _PyRun_SimpleStringFlagsNoPrint and make _PyRun_SimpleStringFlagsWithName a thin wrapper similarly. - Rename single-letter variable 'v' and 'res' to 'result' throughout. - Add test cases for sys.exit() handling via -c, file, -m and message.
Contributor
Author
Added 5 test cases in Lib/test/test_runpy.py covering sys.exit() via -c, file, -m, default exit code, and exit with message. |
cglouch
reviewed
Jun 26, 2026
cglouch
reviewed
Jun 26, 2026
Add test_run_main_system_exit_module and test_run_main_system_exit_command_message to _testembed.c and test_embed.py, covering the remaining sys.exit() paths mentioned in the review feedback (-m module and string message variants).
Member
|
Hi. Thanks for your interesting fix! I wrote a different approach in PR gh-153294: instead of returning a |
Member
|
The legacy REPL of pymain_run_stdin() doesn't handle SystemExit properly: |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Switch to low-level APIs so SystemExit goes through pymain_exit_err_print() instead of exit().