diff --git a/Doc/data/python3.12.abi b/Doc/data/python3.12.abi index ca82d5a3107d70..6abf43ca6a458b 100644 --- a/Doc/data/python3.12.abi +++ b/Doc/data/python3.12.abi @@ -1707,7 +1707,7 @@ - + @@ -2534,14 +2534,14 @@ - + - + - + @@ -7563,19 +7563,19 @@ - + - + - + - + @@ -16067,7 +16067,7 @@ - + @@ -16276,7 +16276,10 @@ - + + + + @@ -16293,18 +16296,18 @@ - + - + - + - + - + @@ -16587,7 +16590,7 @@ - + @@ -16700,10 +16703,13 @@ - + - + + + + @@ -17463,7 +17469,7 @@ - + @@ -22067,11 +22073,11 @@ - + - + @@ -23504,7 +23510,7 @@ - + @@ -24753,7 +24759,7 @@ - + @@ -24945,7 +24951,7 @@ - + @@ -25134,24 +25140,24 @@ - + - + - + - - + + - + @@ -25489,7 +25495,7 @@ - + diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 1db23145a539cb..2fbb9f1aec6154 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -194,6 +194,10 @@ struct _is { struct _Py_interp_cached_objects cached_objects; struct _Py_interp_static_objects static_objects; + /* The ID of the OS thread in which we are finalizing. + We use _Py_atomic_address instead of adding a new _Py_atomic_ulong. */ + _Py_atomic_address _finalizing_id; + /* the initial PyInterpreterState.threads.head */ PyThreadState _initial_thread; }; @@ -209,9 +213,23 @@ _PyInterpreterState_GetFinalizing(PyInterpreterState *interp) { return (PyThreadState*)_Py_atomic_load_relaxed(&interp->_finalizing); } +static inline unsigned long +_PyInterpreterState_GetFinalizingID(PyInterpreterState *interp) { + return (unsigned long)_Py_atomic_load_relaxed(&interp->_finalizing_id); +} + static inline void _PyInterpreterState_SetFinalizing(PyInterpreterState *interp, PyThreadState *tstate) { _Py_atomic_store_relaxed(&interp->_finalizing, (uintptr_t)tstate); + if (tstate == NULL) { + _Py_atomic_store_relaxed(&interp->_finalizing_id, 0); + } + else { + // XXX Re-enable this assert once gh-109860 is fixed. + //assert(tstate->thread_id == PyThread_get_thread_ident()); + _Py_atomic_store_relaxed(&interp->_finalizing_id, + (uintptr_t)tstate->thread_id); + } } diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 4209d090b77df2..e4186b3a921428 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -36,8 +36,12 @@ _Py_IsMainInterpreter(PyInterpreterState *interp) static inline int _Py_IsMainInterpreterFinalizing(PyInterpreterState *interp) { - return (_PyRuntimeState_GetFinalizing(interp->runtime) != NULL && - interp == &interp->runtime->_main_interpreter); + /* bpo-39877: Access _PyRuntime directly rather than using + tstate->interp->runtime to support calls from Python daemon threads. + After Py_Finalize() has been called, tstate can be a dangling pointer: + point to PyThreadState freed memory. */ + return (_PyRuntimeState_GetFinalizing(&_PyRuntime) != NULL && + interp == &_PyRuntime._main_interpreter); } diff --git a/Include/internal/pycore_runtime.h b/Include/internal/pycore_runtime.h index 9b5e12356018fd..bc972783fc94f1 100644 --- a/Include/internal/pycore_runtime.h +++ b/Include/internal/pycore_runtime.h @@ -163,6 +163,9 @@ typedef struct pyruntimestate { struct _Py_static_objects static_objects; struct _Py_cached_objects cached_objects; + /* The ID of the OS thread in which we are finalizing. + We use _Py_atomic_address instead of adding a new _Py_atomic_ulong. */ + _Py_atomic_address _finalizing_id; /* The value to use for sys.path[0] in new subinterpreters. Normally this would be part of the PyConfig struct. However, we cannot add it there in 3.12 since that's an ABI change. */ @@ -210,9 +213,23 @@ _PyRuntimeState_GetFinalizing(_PyRuntimeState *runtime) { return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->_finalizing); } +static inline unsigned long +_PyRuntimeState_GetFinalizingID(_PyRuntimeState *runtime) { + return (unsigned long)_Py_atomic_load_relaxed(&runtime->_finalizing_id); +} + static inline void _PyRuntimeState_SetFinalizing(_PyRuntimeState *runtime, PyThreadState *tstate) { _Py_atomic_store_relaxed(&runtime->_finalizing, (uintptr_t)tstate); + if (tstate == NULL) { + _Py_atomic_store_relaxed(&runtime->_finalizing_id, 0); + } + else { + // XXX Re-enable this assert once gh-109860 is fixed. + //assert(tstate->thread_id == PyThread_get_thread_ident()); + _Py_atomic_store_relaxed(&runtime->_finalizing_id, + (uintptr_t)tstate->thread_id); + } } #ifdef __cplusplus diff --git a/Lib/test/test_interpreters.py b/Lib/test/test_interpreters.py index b426fa12f3e70e..38cf4b6fe737dd 100644 --- a/Lib/test/test_interpreters.py +++ b/Lib/test/test_interpreters.py @@ -639,6 +639,26 @@ def test_sys_path_0(self): self.assertEqual(sp0_sub, expected) +class FinalizationTests(TestBase): + + def test_gh_109793(self): + import subprocess + argv = [sys.executable, '-c', '''if True: + import _xxsubinterpreters as _interpreters + interpid = _interpreters.create() + raise Exception + '''] + proc = subprocess.run(argv, capture_output=True, text=True) + self.assertIn('Traceback', proc.stderr) + if proc.returncode == 0 and support.verbose: + print() + print("--- cmd unexpected succeeded ---") + print(f"stdout:\n{proc.stdout}") + print(f"stderr:\n{proc.stderr}") + print("------") + self.assertEqual(proc.returncode, 1) + + class TestIsShareable(TestBase): def test_default_shareables(self): diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-25-09-24-10.gh-issue-109793.zFQBkv.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-25-09-24-10.gh-issue-109793.zFQBkv.rst new file mode 100644 index 00000000000000..d2dc4c830a9031 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-25-09-24-10.gh-issue-109793.zFQBkv.rst @@ -0,0 +1,4 @@ +The main thread no longer exits prematurely when a subinterpreter +is cleaned up during runtime finalization. The bug was a problem +particularly because, when triggered, the Python process would +always return with a 0 exitcode, even if it failed. diff --git a/Python/pystate.c b/Python/pystate.c index e789eb9ce39086..0430454432be7b 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2916,11 +2916,26 @@ _PyThreadState_MustExit(PyThreadState *tstate) tstate->interp->runtime to support calls from Python daemon threads. After Py_Finalize() has been called, tstate can be a dangling pointer: point to PyThreadState freed memory. */ + unsigned long finalizing_id = _PyRuntimeState_GetFinalizingID(&_PyRuntime); PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime); if (finalizing == NULL) { + // XXX This isn't completely safe from daemon thraeds, + // since tstate might be a dangling pointer. finalizing = _PyInterpreterState_GetFinalizing(tstate->interp); + finalizing_id = _PyInterpreterState_GetFinalizingID(tstate->interp); } - return (finalizing != NULL && finalizing != tstate); + // XXX else check &_PyRuntime._main_interpreter._initial_thread + if (finalizing == NULL) { + return 0; + } + else if (finalizing == tstate) { + return 0; + } + else if (finalizing_id == PyThread_get_thread_ident()) { + /* gh-109793: we must have switched interpreters. */ + return 0; + } + return 1; }