gh-152132: Handle sys.exit() in Py_RunMain command and file paths#152191
gh-152132: Handle sys.exit() in Py_RunMain command and file paths#152191tangyuan0821 wants to merge 4 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
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.
Added 5 test cases in Lib/test/test_runpy.py covering sys.exit() via -c, file, -m, default exit code, and exit with message. |
| ham = self.ham | ||
| self.assertSigInt(["-m", ham.stem], cwd=ham.parent) | ||
|
|
||
| # Tests for sys.exit() handling (gh-152132) |
There was a problem hiding this comment.
do these tests actually fail before the fix? The issue is specifically when embedding, Py_RunMain (and Py_Main / Py_BytesMain, since they call the latter) exits early instead of returning the exit code to the host, but we won't be able to distinguish that just by running a subprocess; it will have the right exit code either way.
I think we would need a test that actually calls Py_RunMain (one with a command, and another with a file; for the latter, could also just use Py_BytesMain, which is where I ran into the original issue). Maybe Programs/_testembed.c and Lib/test/test_embed.py is the right place? Not sure, I'll defer to the other reviewers who are more familiar with this code.
| /* Use _PyRun_SimpleFileObjectNoPrint which returns PyObject* without calling | ||
| PyErr_Print(), so we can handle SystemExit properly via pymain_exit_err_print. */ | ||
| PyCompilerFlags cf = _PyCompilerFlags_INIT; | ||
| int run = _PyRun_AnyFileObject(fp, filename, 1, &cf); |
There was a problem hiding this comment.
previously _PyRun_AnyFileObject had some logic that checked whether _Py_FdIsInteractive and ran the interactive REPL if so, but now we're bypassing that by calling _PyRun_SimpleFileObjectNoPrint. Do we now need to pull out the _Py_FdIsInteractive check into pymain_run_file_obj here too? Otherwise it looks like e.g. 'python3 /dev/stdin' would behave differently.
3 participants
Switch to low-level APIs so SystemExit goes through pymain_exit_err_print() instead of exit().