From 1aab105ba8d9c9f85c245159e31afe48fb3c861f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 20 Jun 2024 17:30:42 -0600 Subject: [PATCH 1/7] Disallow Py_Finalize() if Py_RunMain() is running. --- Include/internal/pycore_runtime.h | 2 ++ Modules/main.c | 8 +++++++- Python/pylifecycle.c | 33 +++++++++++++++++++++++++++---- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index f58eccf729cb2a..609208b1474c68 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -209,6 +209,8 @@ typedef struct pyruntimestate { unsigned long main_thread; PyThreadState *main_tstate; + int is_pymain; + /* ---------- IMPORTANT --------------------------- The fields above this line are declared as early as possible to facilitate out-of-process observability diff --git a/Modules/main.c b/Modules/main.c index 1a70b300b6ad17..5bd0dddae38c1f 100644 --- a/Modules/main.c +++ b/Modules/main.c @@ -711,19 +711,25 @@ pymain_exit_error(PyStatus status) } +extern int _Py_FinalizeMain(void); + int Py_RunMain(void) { int exitcode = 0; + _PyRuntime.is_pymain = 1; + pymain_run_python(&exitcode); - if (Py_FinalizeEx() < 0) { + if (_Py_FinalizeMain() < 0) { /* Value unlikely to be confused with a non-error exit status or other special meaning */ exitcode = 120; } + _PyRuntime.is_pymain = 0; + pymain_free(); if (_PyRuntime.signals.unhandled_keyboard_interrupt) { diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 3639cf6712053e..423b1629606931 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1909,18 +1909,25 @@ finalize_interp_delete(PyInterpreterState *interp) } -int -Py_FinalizeEx(void) +static int +_Py_Finalize(_PyRuntimeState *runtime, const char *not_pymain) { int status = 0; - _PyRuntimeState *runtime = &_PyRuntime; if (!runtime->initialized) { return status; } + if (not_pymain && runtime->is_pymain) { + fprintf(stderr, + "%s() should not be called while Py_RunMain() is running", + not_pymain); + return 1; + } + /* Get current thread state and interpreter pointer */ PyThreadState *tstate = _PyThreadState_GET(); + assert(tstate->interp->runtime == runtime); // XXX assert(_Py_IsMainInterpreter(tstate->interp)); // XXX assert(_Py_IsMainThread()); @@ -2142,10 +2149,28 @@ Py_FinalizeEx(void) return status; } +/* _Py_FinalizeMain() is used exclusively by Py_RunMain(). */ +int +_Py_FinalizeMain(void) +{ + _PyRuntimeState *runtime = &_PyRuntime; + assert(runtime->is_pymain); + assert(_Py_IsMainInterpreter(_PyInterpreterState_GET())); + assert(_Py_IsMainThread()); + assert(_PyThreadState_GET() == runtime->main_tstate); + return _Py_Finalize(runtime, NULL); +} + +int +Py_FinalizeEx(void) +{ + return _Py_Finalize(&_PyRuntime, "Py_FinalizeEx"); +} + void Py_Finalize(void) { - Py_FinalizeEx(); + (void)_Py_Finalize(&_PyRuntime, "Py_Finalize"); } From 0fee2500f4c76bbab32bf896a35a71cf0b4be09f Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 21 Jun 2024 09:56:25 -0600 Subject: [PATCH 2/7] Add a NEWS entry. --- .../next/C API/2024-06-21-09-55-56.gh-issue-120838.-xuGRW.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/C API/2024-06-21-09-55-56.gh-issue-120838.-xuGRW.rst diff --git a/Misc/NEWS.d/next/C API/2024-06-21-09-55-56.gh-issue-120838.-xuGRW.rst b/Misc/NEWS.d/next/C API/2024-06-21-09-55-56.gh-issue-120838.-xuGRW.rst new file mode 100644 index 00000000000000..65fac29d9ef8c6 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2024-06-21-09-55-56.gh-issue-120838.-xuGRW.rst @@ -0,0 +1,3 @@ +Calling :c:func:`Py_Finalize` while :c:func:`Py_RunMain` is running is now a +failure. When used, ``Py_RunMain()`` is solely responsible for finalizing +the runtime. From 902d168392f46ac77fd21f666dab16743dcd6dfd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 21 Jun 2024 13:27:31 -0600 Subject: [PATCH 3/7] Add struct pyfinalize_args. --- Include/internal/pycore_pylifecycle.h | 8 ++++++++ Python/pylifecycle.c | 26 +++++++++++++++++++------- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index f426ae0e103b9c..ae0b9de4b6cc6f 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -114,6 +114,14 @@ PyAPI_FUNC(char*) _Py_SetLocaleFromEnv(int category); // Export for special main.c string compiling with source tracebacks int _PyRun_SimpleStringFlagsWithName(const char *command, const char* name, PyCompilerFlags *flags); +struct pyfinalize_args { + const char *caller; + int check_pymain; +}; + +// Export for _testembed +extern int _Py_Finalize(_PyRuntimeState *, struct pyfinalize_args *); + /* interpreter config */ diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 423b1629606931..6613c0b4805ca8 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1909,19 +1909,20 @@ finalize_interp_delete(PyInterpreterState *interp) } -static int -_Py_Finalize(_PyRuntimeState *runtime, const char *not_pymain) +int +_Py_Finalize(_PyRuntimeState *runtime, struct pyfinalize_args *args) { int status = 0; if (!runtime->initialized) { + assert(!runtime->is_pymain); return status; } - if (not_pymain && runtime->is_pymain) { + if (args->check_pymain && runtime->is_pymain) { fprintf(stderr, "%s() should not be called while Py_RunMain() is running", - not_pymain); + args->caller); return 1; } @@ -2158,19 +2159,30 @@ _Py_FinalizeMain(void) assert(_Py_IsMainInterpreter(_PyInterpreterState_GET())); assert(_Py_IsMainThread()); assert(_PyThreadState_GET() == runtime->main_tstate); - return _Py_Finalize(runtime, NULL); + struct pyfinalize_args args = { + .caller = "Py_RunMain", + }; + return _Py_Finalize(runtime, &args); } int Py_FinalizeEx(void) { - return _Py_Finalize(&_PyRuntime, "Py_FinalizeEx"); + struct pyfinalize_args args = { + .caller = "Py_FinalizeEx", + .check_pymain = 1, + }; + return _Py_Finalize(&_PyRuntime, &args); } void Py_Finalize(void) { - (void)_Py_Finalize(&_PyRuntime, "Py_Finalize"); + struct pyfinalize_args args = { + .caller = "Py_Finalize", + .check_pymain = 1, + }; + (void)_Py_Finalize(&_PyRuntime, &args); } From 85c5df3d5703f8cd9dc78ff8a039e45f59ad023a Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 21 Jun 2024 13:35:04 -0600 Subject: [PATCH 4/7] Fix Py_Exit(). --- Python/pylifecycle.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 6613c0b4805ca8..afe6429afb0db0 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -3255,7 +3255,11 @@ Py_Exit(int sts) if (tstate != NULL && _PyThreadState_IsRunningMain(tstate)) { _PyInterpreterState_SetNotRunningMain(tstate->interp); } - if (Py_FinalizeEx() < 0) { + struct pyfinalize_args args = { + .caller = "Py_Exit", + /* We don't worry about checking if Py_RunMain() is running. */ + }; + if (_Py_Finalize(&_PyRuntime, &args) < 0) { sts = 120; } From 97da2621502f876a50d93cb538197d85ef2ce715 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 25 Jun 2024 11:22:32 -0600 Subject: [PATCH 5/7] Add some comments and fix some literals. --- Python/pylifecycle.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index afe6429afb0db0..571714606bb29b 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1914,19 +1914,21 @@ _Py_Finalize(_PyRuntimeState *runtime, struct pyfinalize_args *args) { int status = 0; + /* Bail out early if already finalized. */ if (!runtime->initialized) { assert(!runtime->is_pymain); return status; } + /* Make sure Py_RunMain() users aren't calling Py_Finalize(). */ if (args->check_pymain && runtime->is_pymain) { fprintf(stderr, - "%s() should not be called while Py_RunMain() is running", + "%s() should not be called while Py_RunMain() is running\n", args->caller); - return 1; + return -1; } - /* Get current thread state and interpreter pointer */ + /* Get final thread state pointer. */ PyThreadState *tstate = _PyThreadState_GET(); assert(tstate->interp->runtime == runtime); // XXX assert(_Py_IsMainInterpreter(tstate->interp)); From d5dc13ac900504fee3d3926584fd7d038a2a7819 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 25 Jun 2024 11:25:16 -0600 Subject: [PATCH 6/7] Add a NEWS entry. --- .../next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst diff --git a/Misc/NEWS.d/next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst b/Misc/NEWS.d/next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst new file mode 100644 index 00000000000000..9b5963c8e9d255 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst @@ -0,0 +1,2 @@ +:c:func:`PyFinalize` and :c:func:`PyFinalizeEx` now explicitly fail if you +try to call them while :c:func:`Py_RunMain` is running. From ff17431b4974eaec6b5ab3964c4848b9845af837 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Tue, 25 Jun 2024 11:54:53 -0600 Subject: [PATCH 7/7] Revert "Add a NEWS entry." This reverts commit d5dc13ac900504fee3d3926584fd7d038a2a7819. --- .../next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 Misc/NEWS.d/next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst diff --git a/Misc/NEWS.d/next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst b/Misc/NEWS.d/next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst deleted file mode 100644 index 9b5963c8e9d255..00000000000000 --- a/Misc/NEWS.d/next/C API/2024-06-25-11-25-09.gh-issue-120838.hOb-9v.rst +++ /dev/null @@ -1,2 +0,0 @@ -:c:func:`PyFinalize` and :c:func:`PyFinalizeEx` now explicitly fail if you -try to call them while :c:func:`Py_RunMain` is running.