-
-
Notifications
You must be signed in to change notification settings - Fork 34.8k
gh-152132: Handle sys.exit() in Py_RunMain command and file paths #152191
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
88d4cd7
220349e
e88db2f
1f39519
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Fix :c:func:`Py_RunMain` incorrectly calling ``exit()`` on | ||
| :exc:`SystemExit` when running a command or file. The exit code | ||
| is now returned to the caller. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ | |
| #include "pycore_pathconfig.h" // _PyPathConfig_ComputeSysPath0() | ||
| #include "pycore_pylifecycle.h" // _Py_PreInitializeFromPyArgv() | ||
| #include "pycore_pystate.h" // _PyInterpreterState_GET() | ||
| #include "pycore_pythonrun.h" // _PyRun_AnyFileObject() | ||
| #include "pycore_pythonrun.h" // _PyRun_SimpleFileObjectNoPrint() | ||
| #include "pycore_tuple.h" // _PyTuple_FromPair | ||
| #include "pycore_unicodeobject.h" // _PyUnicode_Dedent() | ||
|
|
||
|
|
@@ -235,7 +235,6 @@ static int | |
| pymain_run_command(wchar_t *command) | ||
| { | ||
| PyObject *unicode, *bytes; | ||
| int ret; | ||
|
|
||
| unicode = PyUnicode_FromWideChar(command, -1); | ||
| if (unicode == NULL) { | ||
|
|
@@ -259,9 +258,13 @@ pymain_run_command(wchar_t *command) | |
|
|
||
| PyCompilerFlags cf = _PyCompilerFlags_INIT; | ||
| cf.cf_flags |= PyCF_IGNORE_COOKIE; | ||
| ret = _PyRun_SimpleStringFlagsWithName(PyBytes_AsString(bytes), "<string>", &cf); | ||
| PyObject *result = _PyRun_SimpleStringFlagsNoPrint(PyBytes_AsString(bytes), "<string>", &cf); | ||
| Py_DECREF(bytes); | ||
| return (ret != 0); | ||
| if (result == NULL) { | ||
| return pymain_exit_err_print(); | ||
| } | ||
| Py_DECREF(result); | ||
| return 0; | ||
|
|
||
| error: | ||
| PySys_WriteStderr("Unable to decode the command from the command line:\n"); | ||
|
|
@@ -406,10 +409,15 @@ pymain_run_file_obj(PyObject *program_name, PyObject *filename, | |
| return pymain_exit_err_print(); | ||
| } | ||
|
|
||
| /* PyRun_AnyFileExFlags(closeit=1) calls fclose(fp) before running 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| return (run != 0); | ||
| PyObject *result = _PyRun_SimpleFileObjectNoPrint(fp, filename, 1, &cf); | ||
| if (result == NULL) { | ||
| return pymain_exit_err_print(); | ||
| } | ||
| Py_DECREF(result); | ||
| return 0; | ||
| } | ||
|
|
||
| static int | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.