From 354e6b5458eb9cbdf9454a120a62e373ea0a40e8 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 26 Jun 2025 17:45:07 -0400 Subject: [PATCH 01/21] Execute early finalization handlers in a loop --- Include/internal/pycore_ceval.h | 2 +- Include/internal/pycore_pylifecycle.h | 2 +- Lib/threading.py | 4 +- Modules/_threadmodule.c | 6 ++- Modules/atexitmodule.c | 12 ++++-- Python/ceval_gil.c | 35 ++++++++++++--- Python/pylifecycle.c | 62 ++++++++++++++++++--------- 7 files changed, 86 insertions(+), 37 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index cc2defbdf77821..613591362eb26d 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -43,7 +43,7 @@ PyAPI_FUNC(int) _PyEval_MakePendingCalls(PyThreadState *); # define Py_DEFAULT_RECURSION_LIMIT 1000 #endif -extern void _Py_FinishPendingCalls(PyThreadState *tstate); +extern int _Py_FinishPendingCalls(PyThreadState *tstate); extern void _PyEval_InitState(PyInterpreterState *); extern void _PyEval_SignalReceived(void); diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index 6e89ca33e4208c..71b7936f98a77e 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -85,7 +85,7 @@ extern void _PyErr_DisplayException(PyObject *file, PyObject *exc); extern void _PyThreadState_DeleteCurrent(PyThreadState *tstate); -extern void _PyAtExit_Call(PyInterpreterState *interp); +extern int _PyAtExit_Call(PyInterpreterState *interp); extern int _Py_IsCoreInitialized(void); diff --git a/Lib/threading.py b/Lib/threading.py index b6c451d1fbaabd..b2339489825c9f 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -1558,7 +1558,7 @@ def _shutdown(): # mark it done here. if _main_thread._os_thread_handle.is_done() and _is_main_interpreter(): # _shutdown() was already called - return + return False global _SHUTTING_DOWN _SHUTTING_DOWN = True @@ -1572,7 +1572,7 @@ def _shutdown(): _main_thread._os_thread_handle._set_done() # Wait for all non-daemon threads to exit. - _thread_shutdown() + return _thread_shutdown() def main_thread(): diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 150a266b521736..992abdd9815148 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -2345,6 +2345,7 @@ thread_shutdown(PyObject *self, PyObject *args) { PyThread_ident_t ident = PyThread_get_thread_ident_ex(); thread_module_state *state = get_thread_state(self); + int found_thread = 0; for (;;) { ThreadHandle *handle = NULL; @@ -2353,6 +2354,7 @@ thread_shutdown(PyObject *self, PyObject *args) HEAD_LOCK(&_PyRuntime); struct llist_node *node; llist_for_each_safe(node, &state->shutdown_handles) { + found_thread = 1; ThreadHandle *cur = llist_data(node, ThreadHandle, shutdown_node); if (cur->ident != ident) { ThreadHandle_incref(cur); @@ -2373,13 +2375,13 @@ thread_shutdown(PyObject *self, PyObject *args) PyErr_FormatUnraisable("Exception ignored while joining a thread " "in _thread._shutdown()"); ThreadHandle_decref(handle); - Py_RETURN_NONE; + return PyBool_FromLong(found_thread); } ThreadHandle_decref(handle); } - Py_RETURN_NONE; + return PyBool_FromLong(found_thread); } PyDoc_STRVAR(shutdown_doc, diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 4b068967a6ca6e..3564f43d5167df 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -99,7 +99,7 @@ _PyAtExit_Fini(PyInterpreterState *interp) } } -static void +static int atexit_callfuncs(struct atexit_state *state) { assert(!PyErr_Occurred()); @@ -112,10 +112,13 @@ atexit_callfuncs(struct atexit_state *state) { PyErr_FormatUnraisable("Exception ignored while " "copying atexit callbacks"); - return; + return 0; } + int called = 0; + for (Py_ssize_t i = 0; i < PyList_GET_SIZE(copy); ++i) { + called = 1; // We don't have to worry about evil borrowed references, because // no other threads can access this list. PyObject *tuple = PyList_GET_ITEM(copy, i); @@ -141,14 +144,15 @@ atexit_callfuncs(struct atexit_state *state) atexit_cleanup(state); assert(!PyErr_Occurred()); + return called; } -void +int _PyAtExit_Call(PyInterpreterState *interp) { struct atexit_state *state = &interp->atexit; - atexit_callfuncs(state); + return atexit_callfuncs(state); } diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index aa68371ac8febf..5228054341ef0e 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -837,10 +837,11 @@ handle_signals(PyThreadState *tstate) } static int -_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending) +_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_called) { int res = 0; int32_t npending = -1; + int called = 0; assert(sizeof(pending->max) <= sizeof(size_t) && ((size_t)pending->max) <= Py_ARRAY_LENGTH(pending->calls)); @@ -868,6 +869,8 @@ _make_pending_calls(struct _pending_calls *pending, int32_t *p_npending) break; } + called = 1; + /* having released the lock, perform the callback */ res = func(arg); if ((flags & _Py_PENDING_RAWFREE) && arg != NULL) { @@ -881,6 +884,7 @@ _make_pending_calls(struct _pending_calls *pending, int32_t *p_npending) finally: *p_npending = npending; + *p_called = called; return res; } @@ -917,7 +921,7 @@ clear_pending_handling_thread(struct _pending_calls *pending) } static int -make_pending_calls(PyThreadState *tstate) +make_pending_calls_with_count(PyThreadState *tstate) { PyInterpreterState *interp = tstate->interp; struct _pending_calls *pending = &interp->ceval.pending; @@ -947,7 +951,8 @@ make_pending_calls(PyThreadState *tstate) unsignal_pending_calls(tstate, interp); int32_t npending; - if (_make_pending_calls(pending, &npending) != 0) { + int called; + if (_make_pending_calls(pending, &npending, &called) != 0) { clear_pending_handling_thread(pending); /* There might not be more calls to make, but we play it safe. */ signal_pending_calls(tstate, interp); @@ -958,8 +963,9 @@ make_pending_calls(PyThreadState *tstate) signal_pending_calls(tstate, interp); } + int main_called = 0; if (_Py_IsMainThread() && _Py_IsMainInterpreter(interp)) { - if (_make_pending_calls(pending_main, &npending) != 0) { + if (_make_pending_calls(pending_main, &npending, &main_called) != 0) { clear_pending_handling_thread(pending); /* There might not be more calls to make, but we play it safe. */ signal_pending_calls(tstate, interp); @@ -972,6 +978,16 @@ make_pending_calls(PyThreadState *tstate) } clear_pending_handling_thread(pending); + return Py_MAX(called, main_called); +} + +static int +make_pending_calls(PyThreadState *tstate) +{ + if (make_pending_calls_with_count(tstate) < 0) { + return -1; + } + return 0; } @@ -994,7 +1010,7 @@ _Py_unset_eval_breaker_bit_all(PyInterpreterState *interp, uintptr_t bit) _Py_FOR_EACH_TSTATE_END(interp); } -void +int _Py_FinishPendingCalls(PyThreadState *tstate) { _Py_AssertHoldsTstate(); @@ -1011,13 +1027,18 @@ _Py_FinishPendingCalls(PyThreadState *tstate) #ifndef NDEBUG int32_t npending_prev = INT32_MAX; #endif + int called = 0; do { - if (make_pending_calls(tstate) < 0) { + int res = make_pending_calls_with_count(tstate); + if (res < 0) { PyObject *exc = _PyErr_GetRaisedException(tstate); PyErr_BadInternalCall(); _PyErr_ChainExceptions1(exc); _PyErr_Print(tstate); } + if (res != 0) { + called = 1; + } npending = _Py_atomic_load_int32_relaxed(&pending->npending); if (pending_main != NULL) { @@ -1028,6 +1049,8 @@ _Py_FinishPendingCalls(PyThreadState *tstate) npending_prev = npending; #endif } while (npending > 0); + + return called; } int diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 724fda63511282..f65927b1075f21 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -96,7 +96,7 @@ static PyStatus init_android_streams(PyThreadState *tstate); #if defined(__APPLE__) && HAS_APPLE_SYSTEM_LOG static PyStatus init_apple_streams(PyThreadState *tstate); #endif -static void wait_for_thread_shutdown(PyThreadState *tstate); +static int wait_for_thread_shutdown(PyThreadState *tstate); static void finalize_subinterpreters(void); static void call_ll_exitfuncs(_PyRuntimeState *runtime); @@ -2003,6 +2003,39 @@ resolve_final_tstate(_PyRuntimeState *runtime) return main_tstate; } +static void +make_pre_finalization_calls(PyThreadState *tstate) +{ + /* Each of these functions can start one another, e.g. a pending call + * could start a thread or vice versa. To ensure that we properly clean + * call everything, we run these in a loop until none of them run anything. */ + for (;;) { + int called = 0; + + // Wrap up existing "threading"-module-created, non-daemon threads. + called = Py_MAX(called, wait_for_thread_shutdown(tstate)); + + // Make any remaining pending calls. + called = Py_MAX(called, _Py_FinishPendingCalls(tstate)); + + /* The interpreter is still entirely intact at this point, and the + * exit funcs may be relying on that. In particular, if some thread + * or exit func is still waiting to do an import, the import machinery + * expects Py_IsInitialized() to return true. So don't say the + * runtime is uninitialized until after the exit funcs have run. + * Note that Threading.py uses an exit func to do a join on all the + * threads created thru it, so this also protects pending imports in + * the threads created via Threading. + */ + + called = Py_MAX(called, _PyAtExit_Call(tstate->interp)); + + if (called == 0) { + break; + } + } +} + static int _Py_Finalize(_PyRuntimeState *runtime) { @@ -2019,23 +2052,7 @@ _Py_Finalize(_PyRuntimeState *runtime) // Block some operations. tstate->interp->finalizing = 1; - // Wrap up existing "threading"-module-created, non-daemon threads. - wait_for_thread_shutdown(tstate); - - // Make any remaining pending calls. - _Py_FinishPendingCalls(tstate); - - /* The interpreter is still entirely intact at this point, and the - * exit funcs may be relying on that. In particular, if some thread - * or exit func is still waiting to do an import, the import machinery - * expects Py_IsInitialized() to return true. So don't say the - * runtime is uninitialized until after the exit funcs have run. - * Note that Threading.py uses an exit func to do a join on all the - * threads created thru it, so this also protects pending imports in - * the threads created via Threading. - */ - - _PyAtExit_Call(tstate->interp); + make_pre_finalization_calls(tstate); assert(_PyThreadState_GET() == tstate); @@ -3442,7 +3459,7 @@ Py_ExitStatusException(PyStatus status) the threading module was imported in the first place. The shutdown routine will wait until all non-daemon "threading" threads have completed. */ -static void +static int wait_for_thread_shutdown(PyThreadState *tstate) { PyObject *result; @@ -3452,16 +3469,19 @@ wait_for_thread_shutdown(PyThreadState *tstate) PyErr_FormatUnraisable("Exception ignored on threading shutdown"); } /* else: threading not imported */ - return; + return 0; } + int called = 0; result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown)); if (result == NULL) { PyErr_FormatUnraisable("Exception ignored on threading shutdown"); } else { - Py_DECREF(result); + assert(PyBool_Check(result) && _Py_IsImmortal(result)); + called = result == Py_True; } Py_DECREF(threading); + return called; } int Py_AtExit(void (*func)(void)) From 309052447fc0fbcd6cae4cbd1c0ed1c585a21a87 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 26 Jun 2025 18:17:38 -0400 Subject: [PATCH 02/21] Store Py_MAX result in a variable. It re-evaluates it, apparently. --- Lib/threading.py | 5 +++-- Python/pylifecycle.c | 9 ++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Lib/threading.py b/Lib/threading.py index b2339489825c9f..44477ec7885f46 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -1557,8 +1557,9 @@ def _shutdown(): # normally - that won't happen until the interpreter is nearly dead. So # mark it done here. if _main_thread._os_thread_handle.is_done() and _is_main_interpreter(): - # _shutdown() was already called - return False + # _shutdown() was already called, but threads might have started + # in the meantime. + return _thread_shutdown() global _SHUTTING_DOWN _SHUTTING_DOWN = True diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index f65927b1075f21..57503c2b7d7804 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2013,10 +2013,12 @@ make_pre_finalization_calls(PyThreadState *tstate) int called = 0; // Wrap up existing "threading"-module-created, non-daemon threads. - called = Py_MAX(called, wait_for_thread_shutdown(tstate)); + int threads_joined = wait_for_thread_shutdown(tstate); + called = Py_MAX(called, threads_joined); // Make any remaining pending calls. - called = Py_MAX(called, _Py_FinishPendingCalls(tstate)); + int made_pending_calls = _Py_FinishPendingCalls(tstate); + called = Py_MAX(called, made_pending_calls); /* The interpreter is still entirely intact at this point, and the * exit funcs may be relying on that. In particular, if some thread @@ -2028,7 +2030,8 @@ make_pre_finalization_calls(PyThreadState *tstate) * the threads created via Threading. */ - called = Py_MAX(called, _PyAtExit_Call(tstate->interp)); + int called_atexit = _PyAtExit_Call(tstate->interp); + called = Py_MAX(called, called_atexit); if (called == 0) { break; From 43038b83c4642a2c555922e6fcc44e09f8672528 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 26 Jun 2025 18:24:09 -0400 Subject: [PATCH 03/21] Add a test in test_atexit. --- Lib/test/test_atexit.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index eb01da6e88a8bc..ae1d4bb375e88b 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -79,6 +79,28 @@ def thready(): # want them to affect the rest of the tests. script_helper.assert_python_ok("-c", textwrap.dedent(source)) + @threading_helper.requires_working_threading() + def test_thread_created_in_atexit(self): + source = """ + import atexit + import threading + import time + + + def run(): + print(24) + time.sleep(1) + print(42) + + @atexit.register + def start_thread(): + threading.Thread(target=run).start() + """ + + _, stdout, stderr = script_helper.assert_python_ok("-c", textwrap.dedent(source)) + self.assertEqual(stderr, b"") + self.assertEqual(stdout, b"24\n42\n") + @support.cpython_only class SubinterpreterTest(unittest.TestCase): From 8d4151cc3804a05f2ff3592e64ce478f56db2deb Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 26 Jun 2025 18:24:55 -0400 Subject: [PATCH 04/21] Deal with it in Py_EndInterpreter() as well. --- Python/pylifecycle.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 57503c2b7d7804..897a188cfe1b75 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2455,13 +2455,7 @@ Py_EndInterpreter(PyThreadState *tstate) } interp->finalizing = 1; - // Wrap up existing "threading"-module-created, non-daemon threads. - wait_for_thread_shutdown(tstate); - - // Make any remaining pending calls. - _Py_FinishPendingCalls(tstate); - - _PyAtExit_Call(tstate->interp); + make_pre_finalization_calls(tstate); if (tstate != interp->threads.head || tstate->next != NULL) { Py_FatalError("not the last thread"); From add4d33be20ce0f89dab003a7e10ef53f400407d Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 26 Jun 2025 18:44:48 -0400 Subject: [PATCH 05/21] Add a blurb entry. --- .../2025-06-26-18-44-34.gh-issue-136003.sln51d.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-06-26-18-44-34.gh-issue-136003.sln51d.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-26-18-44-34.gh-issue-136003.sln51d.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-26-18-44-34.gh-issue-136003.sln51d.rst new file mode 100644 index 00000000000000..a1344d2264f665 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-26-18-44-34.gh-issue-136003.sln51d.rst @@ -0,0 +1,3 @@ +Fix :class:`threading.Thread` objects becoming incorrectly daemon when +created from an :mod:`atexit` callback or a pending call +(:c:func:`Py_AddPendingCall`). From 3edc3a80a5de3d5599a7f2cf2de72722f238161f Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 26 Jun 2025 18:53:30 -0400 Subject: [PATCH 06/21] Check the return code in the test. --- Lib/test/test_atexit.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index ae1d4bb375e88b..eeaa5fc22df212 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -97,7 +97,8 @@ def start_thread(): threading.Thread(target=run).start() """ - _, stdout, stderr = script_helper.assert_python_ok("-c", textwrap.dedent(source)) + return_code, stdout, stderr = script_helper.assert_python_ok("-c", textwrap.dedent(source)) + self.assertEqual(return_code, 0) self.assertEqual(stderr, b"") self.assertEqual(stdout, b"24\n42\n") From ec579185880f9374f3925743f5a5b6d6486dae0d Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 26 Jun 2025 19:02:16 -0400 Subject: [PATCH 07/21] Add a test for pending calls. --- Lib/test/test_capi/test_misc.py | 31 +++++++++++++++++++++++++++++++ Modules/_testcapimodule.c | 11 +++++++++++ 2 files changed, 42 insertions(+) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index ef950f5df04ad3..5ba01bb226eb46 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -22,6 +22,7 @@ from test import support from test.support import MISSING_C_DOCSTRINGS from test.support import import_helper +from test.support import script_helper from test.support import threading_helper from test.support import warnings_helper from test.support import requires_limited_api @@ -1641,6 +1642,36 @@ def subthread(): self.assertEqual(actual, int(interpid)) + @threading_helper.requires_working_threading() + def test_pending_call_creates_thread(self): + source = """ + import _testcapi + import threading + import time + + + def output(): + print(24) + time.sleep(1) + print(42) + + + def callback(): + threading.Thread(target=output).start() + + + def create_pending_call(): + time.sleep(1) + _testcapi.simple_pending_call(callback) + + + threading.Thread(target=create_pending_call).start() + """ + return_code, stdout, stderr = script_helper.assert_python_ok('-c', textwrap.dedent(source)) + self.assertEqual(return_code, 0) + self.assertEqual(stdout, b"24\n42\n") + self.assertEqual(stderr, b"") + class SubinterpreterTest(unittest.TestCase): diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 71fffedee146fa..b20b5f0d55080c 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2546,6 +2546,16 @@ toggle_reftrace_printer(PyObject *ob, PyObject *arg) Py_RETURN_NONE; } +static PyObject * +simple_pending_call(PyObject *self, PyObject *callable) +{ + if (Py_AddPendingCall(_pending_callback, Py_NewRef(callable)) < 0) { + return NULL; + } + + Py_RETURN_NONE; +} + static PyMethodDef TestMethods[] = { {"set_errno", set_errno, METH_VARARGS}, {"test_config", test_config, METH_NOARGS}, @@ -2640,6 +2650,7 @@ static PyMethodDef TestMethods[] = { {"test_atexit", test_atexit, METH_NOARGS}, {"code_offset_to_line", _PyCFunction_CAST(code_offset_to_line), METH_FASTCALL}, {"toggle_reftrace_printer", toggle_reftrace_printer, METH_O}, + {"simple_pending_call", simple_pending_call, METH_O}, {NULL, NULL} /* sentinel */ }; From 970153bb98a6a83193de8fe5d436b219f01c34ec Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 26 Jun 2025 19:40:42 -0400 Subject: [PATCH 08/21] Fix tests on Windows. --- Lib/test/test_atexit.py | 3 ++- Lib/test/test_capi/test_misc.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index eeaa5fc22df212..6eabd9c736fdbd 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -99,8 +99,9 @@ def start_thread(): return_code, stdout, stderr = script_helper.assert_python_ok("-c", textwrap.dedent(source)) self.assertEqual(return_code, 0) + end = "\r\n" if os.name == "nt" else "\n" + self.assertEqual(stdout, f"24{end}42{end}".encode("utf-8")) self.assertEqual(stderr, b"") - self.assertEqual(stdout, b"24\n42\n") @support.cpython_only diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 5ba01bb226eb46..2a4f14a74101c1 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -1669,7 +1669,8 @@ def create_pending_call(): """ return_code, stdout, stderr = script_helper.assert_python_ok('-c', textwrap.dedent(source)) self.assertEqual(return_code, 0) - self.assertEqual(stdout, b"24\n42\n") + end = "\r\n" if os.name == "nt" else "\n" + self.assertEqual(stdout, f"24{end}42{end}".encode("utf-8")) self.assertEqual(stderr, b"") From cfd62b84019dbf55aa8c42cdf5c76ed200000702 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 3 Jul 2025 10:15:01 -0400 Subject: [PATCH 09/21] Use os.linesep. --- Lib/test/test_atexit.py | 3 +-- Lib/test/test_capi/test_misc.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index 6eabd9c736fdbd..a273ee49faf687 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -99,8 +99,7 @@ def start_thread(): return_code, stdout, stderr = script_helper.assert_python_ok("-c", textwrap.dedent(source)) self.assertEqual(return_code, 0) - end = "\r\n" if os.name == "nt" else "\n" - self.assertEqual(stdout, f"24{end}42{end}".encode("utf-8")) + self.assertEqual(stdout, f"24{os.linesep}42{os.linesep}".encode("utf-8")) self.assertEqual(stderr, b"") diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 2a4f14a74101c1..918ccda18debe6 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -1669,8 +1669,7 @@ def create_pending_call(): """ return_code, stdout, stderr = script_helper.assert_python_ok('-c', textwrap.dedent(source)) self.assertEqual(return_code, 0) - end = "\r\n" if os.name == "nt" else "\n" - self.assertEqual(stdout, f"24{end}42{end}".encode("utf-8")) + self.assertEqual(stdout, f"24{os.linesep}42{os.linesep}".encode("utf-8")) self.assertEqual(stderr, b"") From 859070f1f3118042fdff6aeac5f8fa93354991b7 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 3 Jul 2025 15:28:26 -0400 Subject: [PATCH 10/21] Use an RW lock instead of a counter. --- Include/internal/pycore_interp_structs.h | 4 ++++ Include/internal/pycore_pylifecycle.h | 2 +- Lib/threading.py | 2 +- Modules/_threadmodule.c | 23 +++++++++++--------- Modules/atexitmodule.c | 20 ++++++++++-------- Python/ceval_gil.c | 27 ++++++++++-------------- Python/pylifecycle.c | 25 ++++++++-------------- 7 files changed, 50 insertions(+), 53 deletions(-) diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index f1f427d99dea69..10a11f626712fc 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -971,6 +971,10 @@ struct _is { # endif #endif + /* The "pre-finalization" lock, which protects against things like starting + * threads. The exclusive writer is only used when the interpreter finalizes. */ + _PyRWMutex prefini_mutex; + /* the initial PyInterpreterState.threads.head */ _PyThreadStateImpl _initial_thread; // _initial_thread should be the last field of PyInterpreterState. diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index 71b7936f98a77e..6e89ca33e4208c 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -85,7 +85,7 @@ extern void _PyErr_DisplayException(PyObject *file, PyObject *exc); extern void _PyThreadState_DeleteCurrent(PyThreadState *tstate); -extern int _PyAtExit_Call(PyInterpreterState *interp); +extern void _PyAtExit_Call(PyInterpreterState *interp); extern int _Py_IsCoreInitialized(void); diff --git a/Lib/threading.py b/Lib/threading.py index 44477ec7885f46..f28dea284a6b48 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -1573,7 +1573,7 @@ def _shutdown(): _main_thread._os_thread_handle._set_done() # Wait for all non-daemon threads to exit. - return _thread_shutdown() + _thread_shutdown() def main_thread(): diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 992abdd9815148..a2ed8cb1c41789 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -176,11 +176,13 @@ ThreadHandle_get_os_handle(ThreadHandle *handle, PyThread_handle_t *os_handle) } static void -add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle) +add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle, PyInterpreterState *interp) { + _PyRWMutex_RLock(&interp->prefini_mutex); HEAD_LOCK(&_PyRuntime); llist_insert_tail(&state->shutdown_handles, &handle->shutdown_node); HEAD_UNLOCK(&_PyRuntime); + _PyRWMutex_RUnlock(&interp->prefini_mutex); } static void @@ -195,13 +197,16 @@ clear_shutdown_handles(thread_module_state *state) } static void -remove_from_shutdown_handles(ThreadHandle *handle) +remove_from_shutdown_handles(ThreadHandle *handle, PyInterpreterState *interp) { + assert(interp != NULL); + _PyRWMutex_RLock(&interp->prefini_mutex); HEAD_LOCK(&_PyRuntime); if (handle->shutdown_node.next != NULL) { llist_remove(&handle->shutdown_node); } HEAD_UNLOCK(&_PyRuntime); + _PyRWMutex_RUnlock(&interp->prefini_mutex); } static ThreadHandle * @@ -309,7 +314,7 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state) handle->mutex = (PyMutex){_Py_UNLOCKED}; _PyEvent_Notify(&handle->thread_is_exiting); llist_remove(node); - remove_from_shutdown_handles(handle); + remove_from_shutdown_handles(handle, _PyInterpreterState_GET()); } } @@ -392,7 +397,7 @@ thread_run(void *boot_raw) exit: // Don't need to wait for this thread anymore - remove_from_shutdown_handles(handle); + remove_from_shutdown_handles(handle, _PyInterpreterState_GET()); _PyEvent_Notify(&handle->thread_is_exiting); ThreadHandle_decref(handle); @@ -1863,12 +1868,12 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args, // Add the handle before starting the thread to avoid adding a handle // to a thread that has already finished (i.e. if the thread finishes // before the call to `ThreadHandle_start()` below returns). - add_to_shutdown_handles(state, handle); + add_to_shutdown_handles(state, handle, interp); } if (ThreadHandle_start(handle, func, args, kwargs) < 0) { if (!daemon) { - remove_from_shutdown_handles(handle); + remove_from_shutdown_handles(handle, _PyInterpreterState_GET()); } return -1; } @@ -2345,7 +2350,6 @@ thread_shutdown(PyObject *self, PyObject *args) { PyThread_ident_t ident = PyThread_get_thread_ident_ex(); thread_module_state *state = get_thread_state(self); - int found_thread = 0; for (;;) { ThreadHandle *handle = NULL; @@ -2354,7 +2358,6 @@ thread_shutdown(PyObject *self, PyObject *args) HEAD_LOCK(&_PyRuntime); struct llist_node *node; llist_for_each_safe(node, &state->shutdown_handles) { - found_thread = 1; ThreadHandle *cur = llist_data(node, ThreadHandle, shutdown_node); if (cur->ident != ident) { ThreadHandle_incref(cur); @@ -2375,13 +2378,13 @@ thread_shutdown(PyObject *self, PyObject *args) PyErr_FormatUnraisable("Exception ignored while joining a thread " "in _thread._shutdown()"); ThreadHandle_decref(handle); - return PyBool_FromLong(found_thread); + Py_RETURN_NONE; } ThreadHandle_decref(handle); } - return PyBool_FromLong(found_thread); + Py_RETURN_NONE; } PyDoc_STRVAR(shutdown_doc, diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 3564f43d5167df..3216506c64bb61 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -99,7 +99,7 @@ _PyAtExit_Fini(PyInterpreterState *interp) } } -static int +static void atexit_callfuncs(struct atexit_state *state) { assert(!PyErr_Occurred()); @@ -112,13 +112,10 @@ atexit_callfuncs(struct atexit_state *state) { PyErr_FormatUnraisable("Exception ignored while " "copying atexit callbacks"); - return 0; + return; } - int called = 0; - for (Py_ssize_t i = 0; i < PyList_GET_SIZE(copy); ++i) { - called = 1; // We don't have to worry about evil borrowed references, because // no other threads can access this list. PyObject *tuple = PyList_GET_ITEM(copy, i); @@ -144,15 +141,14 @@ atexit_callfuncs(struct atexit_state *state) atexit_cleanup(state); assert(!PyErr_Occurred()); - return called; } -int +void _PyAtExit_Call(PyInterpreterState *interp) { struct atexit_state *state = &interp->atexit; - return atexit_callfuncs(state); + atexit_callfuncs(state); } @@ -200,9 +196,15 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs) return NULL; } + PyInterpreterState *interp = _PyInterpreterState_GET(); + assert(interp != NULL); struct atexit_state *state = get_atexit_state(); + _PyRWMutex_RLock(&interp->prefini_mutex); // atexit callbacks go in a LIFO order - if (PyList_Insert(state->callbacks, 0, callback) < 0) + int res = PyList_Insert(state->callbacks, 0, callback); + _PyRWMutex_RUnlock(&interp->prefini_mutex); + + if (res < 0) { Py_DECREF(callback); return NULL; diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 5228054341ef0e..328870f8db10b9 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -785,8 +785,10 @@ _PyEval_AddPendingCall(PyInterpreterState *interp, } PyMutex_Lock(&pending->mutex); + _PyRWMutex_RLock(&interp->prefini_mutex); _Py_add_pending_call_result result = _push_pending_call(pending, func, arg, flags); + _PyRWMutex_RUnlock(&interp->prefini_mutex); PyMutex_Unlock(&pending->mutex); if (main_only) { @@ -837,11 +839,10 @@ handle_signals(PyThreadState *tstate) } static int -_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_called) +_make_pending_calls(struct _pending_calls *pending, int32_t *p_npending) { int res = 0; int32_t npending = -1; - int called = 0; assert(sizeof(pending->max) <= sizeof(size_t) && ((size_t)pending->max) <= Py_ARRAY_LENGTH(pending->calls)); @@ -869,8 +870,6 @@ _make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_ break; } - called = 1; - /* having released the lock, perform the callback */ res = func(arg); if ((flags & _Py_PENDING_RAWFREE) && arg != NULL) { @@ -884,7 +883,6 @@ _make_pending_calls(struct _pending_calls *pending, int32_t *p_npending, int *p_ finally: *p_npending = npending; - *p_called = called; return res; } @@ -921,7 +919,7 @@ clear_pending_handling_thread(struct _pending_calls *pending) } static int -make_pending_calls_with_count(PyThreadState *tstate) +make_pending_calls_lock_held(PyThreadState *tstate) { PyInterpreterState *interp = tstate->interp; struct _pending_calls *pending = &interp->ceval.pending; @@ -951,8 +949,7 @@ make_pending_calls_with_count(PyThreadState *tstate) unsignal_pending_calls(tstate, interp); int32_t npending; - int called; - if (_make_pending_calls(pending, &npending, &called) != 0) { + if (_make_pending_calls(pending, &npending) != 0) { clear_pending_handling_thread(pending); /* There might not be more calls to make, but we play it safe. */ signal_pending_calls(tstate, interp); @@ -963,9 +960,8 @@ make_pending_calls_with_count(PyThreadState *tstate) signal_pending_calls(tstate, interp); } - int main_called = 0; if (_Py_IsMainThread() && _Py_IsMainInterpreter(interp)) { - if (_make_pending_calls(pending_main, &npending, &main_called) != 0) { + if (_make_pending_calls(pending_main, &npending) != 0) { clear_pending_handling_thread(pending); /* There might not be more calls to make, but we play it safe. */ signal_pending_calls(tstate, interp); @@ -978,17 +974,16 @@ make_pending_calls_with_count(PyThreadState *tstate) } clear_pending_handling_thread(pending); - return Py_MAX(called, main_called); + return 0; } static int make_pending_calls(PyThreadState *tstate) { - if (make_pending_calls_with_count(tstate) < 0) { - return -1; - } - - return 0; + _PyRWMutex_RLock(&tstate->interp->prefini_mutex); + int res = make_pending_calls_lock_held(tstate); + _PyRWMutex_RUnlock(&tstate->interp->prefini_mutex); + return res; } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 897a188cfe1b75..e3d2afe6acf426 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -96,7 +96,7 @@ static PyStatus init_android_streams(PyThreadState *tstate); #if defined(__APPLE__) && HAS_APPLE_SYSTEM_LOG static PyStatus init_apple_streams(PyThreadState *tstate); #endif -static int wait_for_thread_shutdown(PyThreadState *tstate); +static void wait_for_thread_shutdown(PyThreadState *tstate); static void finalize_subinterpreters(void); static void call_ll_exitfuncs(_PyRuntimeState *runtime); @@ -2010,15 +2010,11 @@ make_pre_finalization_calls(PyThreadState *tstate) * could start a thread or vice versa. To ensure that we properly clean * call everything, we run these in a loop until none of them run anything. */ for (;;) { - int called = 0; - // Wrap up existing "threading"-module-created, non-daemon threads. - int threads_joined = wait_for_thread_shutdown(tstate); - called = Py_MAX(called, threads_joined); + wait_for_thread_shutdown(tstate); // Make any remaining pending calls. - int made_pending_calls = _Py_FinishPendingCalls(tstate); - called = Py_MAX(called, made_pending_calls); + _Py_FinishPendingCalls(tstate); /* The interpreter is still entirely intact at this point, and the * exit funcs may be relying on that. In particular, if some thread @@ -2030,9 +2026,9 @@ make_pre_finalization_calls(PyThreadState *tstate) * the threads created via Threading. */ - int called_atexit = _PyAtExit_Call(tstate->interp); - called = Py_MAX(called, called_atexit); + _PyAtExit_Call(tstate->interp); + _PyRWMutex_Unlock(&tstate->interp->prefini_mutex); if (called == 0) { break; } @@ -3456,7 +3452,7 @@ Py_ExitStatusException(PyStatus status) the threading module was imported in the first place. The shutdown routine will wait until all non-daemon "threading" threads have completed. */ -static int +static void wait_for_thread_shutdown(PyThreadState *tstate) { PyObject *result; @@ -3466,19 +3462,16 @@ wait_for_thread_shutdown(PyThreadState *tstate) PyErr_FormatUnraisable("Exception ignored on threading shutdown"); } /* else: threading not imported */ - return 0; + return; } int called = 0; result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown)); if (result == NULL) { PyErr_FormatUnraisable("Exception ignored on threading shutdown"); } - else { - assert(PyBool_Check(result) && _Py_IsImmortal(result)); - called = result == Py_True; - } + Py_XDECREF(result); Py_DECREF(threading); - return called; + return; } int Py_AtExit(void (*func)(void)) From 8a1aa139093bf8a9bd2a87745d4b6abc9734903c Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 3 Jul 2025 15:30:39 -0400 Subject: [PATCH 11/21] Remove more counters. --- Include/internal/pycore_ceval.h | 2 +- Python/ceval_gil.c | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 613591362eb26d..cc2defbdf77821 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -43,7 +43,7 @@ PyAPI_FUNC(int) _PyEval_MakePendingCalls(PyThreadState *); # define Py_DEFAULT_RECURSION_LIMIT 1000 #endif -extern int _Py_FinishPendingCalls(PyThreadState *tstate); +extern void _Py_FinishPendingCalls(PyThreadState *tstate); extern void _PyEval_InitState(PyInterpreterState *); extern void _PyEval_SignalReceived(void); diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 328870f8db10b9..f16ba3683147fb 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -1005,7 +1005,7 @@ _Py_unset_eval_breaker_bit_all(PyInterpreterState *interp, uintptr_t bit) _Py_FOR_EACH_TSTATE_END(interp); } -int +void _Py_FinishPendingCalls(PyThreadState *tstate) { _Py_AssertHoldsTstate(); @@ -1024,16 +1024,12 @@ _Py_FinishPendingCalls(PyThreadState *tstate) #endif int called = 0; do { - int res = make_pending_calls_with_count(tstate); - if (res < 0) { + if (make_pending_calls(tstate) < 0) { PyObject *exc = _PyErr_GetRaisedException(tstate); PyErr_BadInternalCall(); _PyErr_ChainExceptions1(exc); _PyErr_Print(tstate); } - if (res != 0) { - called = 1; - } npending = _Py_atomic_load_int32_relaxed(&pending->npending); if (pending_main != NULL) { @@ -1045,7 +1041,7 @@ _Py_FinishPendingCalls(PyThreadState *tstate) #endif } while (npending > 0); - return called; + return; } int From cbcd552fe43f80be48b83180bf037d8e761b5125 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 9 Jul 2025 16:29:16 -0400 Subject: [PATCH 12/21] Remove old artifacts (again). --- Python/ceval_gil.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index f16ba3683147fb..c0f7f105ac2209 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -1022,7 +1022,6 @@ _Py_FinishPendingCalls(PyThreadState *tstate) #ifndef NDEBUG int32_t npending_prev = INT32_MAX; #endif - int called = 0; do { if (make_pending_calls(tstate) < 0) { PyObject *exc = _PyErr_GetRaisedException(tstate); @@ -1040,8 +1039,6 @@ _Py_FinishPendingCalls(PyThreadState *tstate) npending_prev = npending; #endif } while (npending > 0); - - return; } int From 54613a1c985c721b8677d010707a037ecbc55236 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 9 Jul 2025 16:30:13 -0400 Subject: [PATCH 13/21] Final time removing artifacts. --- Python/pylifecycle.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index e3d2afe6acf426..95eb6ef0ba5f64 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2029,9 +2029,6 @@ make_pre_finalization_calls(PyThreadState *tstate) _PyAtExit_Call(tstate->interp); _PyRWMutex_Unlock(&tstate->interp->prefini_mutex); - if (called == 0) { - break; - } } } @@ -3464,14 +3461,12 @@ wait_for_thread_shutdown(PyThreadState *tstate) /* else: threading not imported */ return; } - int called = 0; result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown)); if (result == NULL) { PyErr_FormatUnraisable("Exception ignored on threading shutdown"); } Py_XDECREF(result); Py_DECREF(threading); - return; } int Py_AtExit(void (*func)(void)) From 9ccdb5fb879e078e84611ccf74855c78b318aa23 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 9 Jul 2025 16:31:17 -0400 Subject: [PATCH 14/21] (I lied) --- Python/pylifecycle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 95eb6ef0ba5f64..36ed16b14ff3fe 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -3464,8 +3464,9 @@ wait_for_thread_shutdown(PyThreadState *tstate) result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown)); if (result == NULL) { PyErr_FormatUnraisable("Exception ignored on threading shutdown"); + } else { + Py_DECREF(result); } - Py_XDECREF(result); Py_DECREF(threading); } From 9cd75b7df5347531d07b35ce7243c561496dccf0 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 9 Jul 2025 16:45:22 -0400 Subject: [PATCH 15/21] Atomically check if there are threads, atexit callbacks, or pending calls. --- Modules/_threadmodule.c | 9 +++++---- Python/pylifecycle.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index fb1964374af863..a8c85ac401c27e 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -347,6 +347,7 @@ thread_run(void *boot_raw) { struct bootstate *boot = (struct bootstate *) boot_raw; PyThreadState *tstate = boot->tstate; + PyInterpreterState *interp = tstate->interp; // Wait until the handle is marked as running PyEvent_Wait(&boot->handle_ready); @@ -373,7 +374,7 @@ thread_run(void *boot_raw) _PyThreadState_Bind(tstate); PyEval_AcquireThread(tstate); - _Py_atomic_add_ssize(&tstate->interp->threads.count, 1); + _Py_atomic_add_ssize(&interp->threads.count, 1); PyObject *res = PyObject_Call(boot->func, boot->args, boot->kwargs); if (res == NULL) { @@ -391,13 +392,13 @@ thread_run(void *boot_raw) thread_bootstate_free(boot, 1); - _Py_atomic_add_ssize(&tstate->interp->threads.count, -1); + _Py_atomic_add_ssize(&interp->threads.count, -1); PyThreadState_Clear(tstate); _PyThreadState_DeleteCurrent(tstate); exit: // Don't need to wait for this thread anymore - remove_from_shutdown_handles(handle, _PyInterpreterState_GET()); + remove_from_shutdown_handles(handle, interp); _PyEvent_Notify(&handle->thread_is_exiting); ThreadHandle_decref(handle); @@ -1881,7 +1882,7 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args, if (ThreadHandle_start(handle, func, args, kwargs) < 0) { if (!daemon) { - remove_from_shutdown_handles(handle, _PyInterpreterState_GET()); + remove_from_shutdown_handles(handle, interp); } return -1; } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 36ed16b14ff3fe..65b1e9e2eb80db 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2003,9 +2003,35 @@ resolve_final_tstate(_PyRuntimeState *runtime) return main_tstate; } +static int +interp_has_threads(PyInterpreterState *interp) +{ + assert(interp != NULL); + assert(interp->threads.head != NULL); + return interp->threads.head->next != NULL; +} + +static int +interp_has_pending_calls(PyInterpreterState *interp) +{ + assert(interp != NULL); + return interp->ceval.pending.npending != 0; +} + +static int +interp_has_atexit_callbacks(PyInterpreterState *interp) +{ + assert(interp != NULL); + assert(interp->atexit.callbacks != NULL); + assert(PyList_CheckExact(interp->atexit.callbacks)); + return PyList_GET_SIZE(interp->atexit.callbacks) != 0; +} + static void make_pre_finalization_calls(PyThreadState *tstate) { + assert(tstate != NULL); + PyInterpreterState *interp = tstate->interp; /* Each of these functions can start one another, e.g. a pending call * could start a thread or vice versa. To ensure that we properly clean * call everything, we run these in a loop until none of them run anything. */ @@ -2028,7 +2054,14 @@ make_pre_finalization_calls(PyThreadState *tstate) _PyAtExit_Call(tstate->interp); + _PyRWMutex_Lock(&tstate->interp->prefini_mutex); + int should_continue = (interp_has_threads(interp) + || interp_has_atexit_callbacks(interp) + || interp_has_pending_calls(interp)); _PyRWMutex_Unlock(&tstate->interp->prefini_mutex); + if (!should_continue) { + break; + } } } From 1360059fe64aaf33e630c282aeb50dc6bb307de8 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 9 Jul 2025 16:47:37 -0400 Subject: [PATCH 16/21] Remove stray newline change. --- Python/pylifecycle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 65b1e9e2eb80db..09e687d183f35d 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -3497,7 +3497,8 @@ wait_for_thread_shutdown(PyThreadState *tstate) result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown)); if (result == NULL) { PyErr_FormatUnraisable("Exception ignored on threading shutdown"); - } else { + } + else { Py_DECREF(result); } Py_DECREF(threading); From 475538aafc809f328ab082049603a4d3655ade4e Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 10 Jul 2025 05:03:16 -0400 Subject: [PATCH 17/21] Add a test for atexit with subinterpreters. --- Lib/test/test_atexit.py | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index a273ee49faf687..ea70df89590869 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -81,7 +81,7 @@ def thready(): @threading_helper.requires_working_threading() def test_thread_created_in_atexit(self): - source = """ + source = """if True: import atexit import threading import time @@ -96,12 +96,46 @@ def run(): def start_thread(): threading.Thread(target=run).start() """ - - return_code, stdout, stderr = script_helper.assert_python_ok("-c", textwrap.dedent(source)) + return_code, stdout, stderr = script_helper.assert_python_ok("-c", source) self.assertEqual(return_code, 0) self.assertEqual(stdout, f"24{os.linesep}42{os.linesep}".encode("utf-8")) self.assertEqual(stderr, b"") + @threading_helper.requires_working_threading() + def test_thread_created_in_atexit_subinterpreter(self): + import os + + try: + from concurrent import interpreters + except ImportError: + self.skipTest("subinterpreters are not available") + + read, write = os.pipe() + source = f"""if True: + import atexit + import threading + import time + import os + + def run(): + os.write({write}, b'spanish') + time.sleep(1) + os.write({write}, b'inquisition') + + @atexit.register + def start_thread(): + threading.Thread(target=run).start() + """ + interp = interpreters.create() + try: + interp.exec(source) + + # Close the interpreter to invoke atexit callbacks + interp.close() + self.assertEqual(os.read(read, 100), b"spanishinquisition") + finally: + os.close(read) + os.close(write) @support.cpython_only class SubinterpreterTest(unittest.TestCase): From a79418839b876291709ab8f979684fbebebe4cba Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 10 Jul 2025 05:07:11 -0400 Subject: [PATCH 18/21] Check for os.pipe() in the test. --- Lib/test/test_atexit.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index ea70df89590869..66142a108d5d93 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -102,9 +102,8 @@ def start_thread(): self.assertEqual(stderr, b"") @threading_helper.requires_working_threading() + @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") def test_thread_created_in_atexit_subinterpreter(self): - import os - try: from concurrent import interpreters except ImportError: From 2dda7a4b4ea4989d0790cf4fb5f28e15fdc8743c Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 10 Jul 2025 05:57:03 -0400 Subject: [PATCH 19/21] Rely on stop-the-world and the GIL instead of a dedicated RW mutex. This solution is much simpler, but will perform a little bit worse. Instead of using a dedicated lock on the interpreter state, we can simply use stop-the-world (and the GIL on the default build) to ensure that no other threads can create pre-finalization callbacks concurrently. --- Include/internal/pycore_interp_structs.h | 4 ---- Modules/_threadmodule.c | 22 ++++++++-------------- Modules/atexitmodule.c | 8 +------- Python/ceval_gil.c | 13 +------------ Python/pylifecycle.c | 12 +++++++++--- 5 files changed, 19 insertions(+), 40 deletions(-) diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index 10a11f626712fc..f1f427d99dea69 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -971,10 +971,6 @@ struct _is { # endif #endif - /* The "pre-finalization" lock, which protects against things like starting - * threads. The exclusive writer is only used when the interpreter finalizes. */ - _PyRWMutex prefini_mutex; - /* the initial PyInterpreterState.threads.head */ _PyThreadStateImpl _initial_thread; // _initial_thread should be the last field of PyInterpreterState. diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index a8c85ac401c27e..8886a9d6bd0c8d 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -176,13 +176,11 @@ ThreadHandle_get_os_handle(ThreadHandle *handle, PyThread_handle_t *os_handle) } static void -add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle, PyInterpreterState *interp) +add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle) { - _PyRWMutex_RLock(&interp->prefini_mutex); HEAD_LOCK(&_PyRuntime); llist_insert_tail(&state->shutdown_handles, &handle->shutdown_node); HEAD_UNLOCK(&_PyRuntime); - _PyRWMutex_RUnlock(&interp->prefini_mutex); } static void @@ -197,16 +195,13 @@ clear_shutdown_handles(thread_module_state *state) } static void -remove_from_shutdown_handles(ThreadHandle *handle, PyInterpreterState *interp) +remove_from_shutdown_handles(ThreadHandle *handle) { - assert(interp != NULL); - _PyRWMutex_RLock(&interp->prefini_mutex); HEAD_LOCK(&_PyRuntime); if (handle->shutdown_node.next != NULL) { llist_remove(&handle->shutdown_node); } HEAD_UNLOCK(&_PyRuntime); - _PyRWMutex_RUnlock(&interp->prefini_mutex); } static ThreadHandle * @@ -314,7 +309,7 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state) handle->mutex = (PyMutex){_Py_UNLOCKED}; _PyEvent_Notify(&handle->thread_is_exiting); llist_remove(node); - remove_from_shutdown_handles(handle, _PyInterpreterState_GET()); + remove_from_shutdown_handles(handle); } } @@ -347,7 +342,6 @@ thread_run(void *boot_raw) { struct bootstate *boot = (struct bootstate *) boot_raw; PyThreadState *tstate = boot->tstate; - PyInterpreterState *interp = tstate->interp; // Wait until the handle is marked as running PyEvent_Wait(&boot->handle_ready); @@ -374,7 +368,7 @@ thread_run(void *boot_raw) _PyThreadState_Bind(tstate); PyEval_AcquireThread(tstate); - _Py_atomic_add_ssize(&interp->threads.count, 1); + _Py_atomic_add_ssize(&tstate->interp->threads.count, 1); PyObject *res = PyObject_Call(boot->func, boot->args, boot->kwargs); if (res == NULL) { @@ -392,13 +386,13 @@ thread_run(void *boot_raw) thread_bootstate_free(boot, 1); - _Py_atomic_add_ssize(&interp->threads.count, -1); + _Py_atomic_add_ssize(&tstate->interp->threads.count, -1); PyThreadState_Clear(tstate); _PyThreadState_DeleteCurrent(tstate); exit: // Don't need to wait for this thread anymore - remove_from_shutdown_handles(handle, interp); + remove_from_shutdown_handles(handle); _PyEvent_Notify(&handle->thread_is_exiting); ThreadHandle_decref(handle); @@ -1877,12 +1871,12 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args, // Add the handle before starting the thread to avoid adding a handle // to a thread that has already finished (i.e. if the thread finishes // before the call to `ThreadHandle_start()` below returns). - add_to_shutdown_handles(state, handle, interp); + add_to_shutdown_handles(state, handle); } if (ThreadHandle_start(handle, func, args, kwargs) < 0) { if (!daemon) { - remove_from_shutdown_handles(handle, interp); + remove_from_shutdown_handles(handle); } return -1; } diff --git a/Modules/atexitmodule.c b/Modules/atexitmodule.c index 3216506c64bb61..4b068967a6ca6e 100644 --- a/Modules/atexitmodule.c +++ b/Modules/atexitmodule.c @@ -196,15 +196,9 @@ atexit_register(PyObject *module, PyObject *args, PyObject *kwargs) return NULL; } - PyInterpreterState *interp = _PyInterpreterState_GET(); - assert(interp != NULL); struct atexit_state *state = get_atexit_state(); - _PyRWMutex_RLock(&interp->prefini_mutex); // atexit callbacks go in a LIFO order - int res = PyList_Insert(state->callbacks, 0, callback); - _PyRWMutex_RUnlock(&interp->prefini_mutex); - - if (res < 0) + if (PyList_Insert(state->callbacks, 0, callback) < 0) { Py_DECREF(callback); return NULL; diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index c0f7f105ac2209..aa68371ac8febf 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -785,10 +785,8 @@ _PyEval_AddPendingCall(PyInterpreterState *interp, } PyMutex_Lock(&pending->mutex); - _PyRWMutex_RLock(&interp->prefini_mutex); _Py_add_pending_call_result result = _push_pending_call(pending, func, arg, flags); - _PyRWMutex_RUnlock(&interp->prefini_mutex); PyMutex_Unlock(&pending->mutex); if (main_only) { @@ -919,7 +917,7 @@ clear_pending_handling_thread(struct _pending_calls *pending) } static int -make_pending_calls_lock_held(PyThreadState *tstate) +make_pending_calls(PyThreadState *tstate) { PyInterpreterState *interp = tstate->interp; struct _pending_calls *pending = &interp->ceval.pending; @@ -977,15 +975,6 @@ make_pending_calls_lock_held(PyThreadState *tstate) return 0; } -static int -make_pending_calls(PyThreadState *tstate) -{ - _PyRWMutex_RLock(&tstate->interp->prefini_mutex); - int res = make_pending_calls_lock_held(tstate); - _PyRWMutex_RUnlock(&tstate->interp->prefini_mutex); - return res; -} - void _Py_set_eval_breaker_bit_all(PyInterpreterState *interp, uintptr_t bit) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 09e687d183f35d..3a6b62a9e4590c 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2036,6 +2036,8 @@ make_pre_finalization_calls(PyThreadState *tstate) * could start a thread or vice versa. To ensure that we properly clean * call everything, we run these in a loop until none of them run anything. */ for (;;) { + assert(!interp->runtime->stoptheworld.world_stopped); + // Wrap up existing "threading"-module-created, non-daemon threads. wait_for_thread_shutdown(tstate); @@ -2054,15 +2056,18 @@ make_pre_finalization_calls(PyThreadState *tstate) _PyAtExit_Call(tstate->interp); - _PyRWMutex_Lock(&tstate->interp->prefini_mutex); + // XXX Why does _PyThreadState_DeleteList() rely on all interpreters + // being stopped? + _PyEval_StopTheWorldAll(interp->runtime); int should_continue = (interp_has_threads(interp) || interp_has_atexit_callbacks(interp) || interp_has_pending_calls(interp)); - _PyRWMutex_Unlock(&tstate->interp->prefini_mutex); if (!should_continue) { break; } + _PyEval_StartTheWorldAll(interp->runtime); } + assert(interp->runtime->stoptheworld.world_stopped); } static int @@ -2099,7 +2104,7 @@ _Py_Finalize(_PyRuntimeState *runtime) #endif /* Ensure that remaining threads are detached */ - _PyEval_StopTheWorldAll(runtime); + assert(tstate->interp->runtime->stoptheworld.world_stopped); /* Remaining daemon threads will be trapped in PyThread_hang_thread when they attempt to take the GIL (ex: PyEval_RestoreThread()). */ @@ -2490,6 +2495,7 @@ Py_EndInterpreter(PyThreadState *tstate) /* Remaining daemon threads will automatically exit when they attempt to take the GIL (ex: PyEval_RestoreThread()). */ _PyInterpreterState_SetFinalizing(interp, tstate); + _PyEval_StartTheWorldAll(interp->runtime); // XXX Call something like _PyImport_Disable() here? From f1460aff03e52ce018147739277628087564ec8f Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 10 Jul 2025 06:05:05 -0400 Subject: [PATCH 20/21] Serialize pending calls via the ceval mutex. Py_AddPendingCall() can be called without an attached thread state, so we can't rely on stop-the-world or the GIL for it. Instead, acquire its dedicated mutex to prevent threads from creating new pending calls. --- Python/pylifecycle.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 3a6b62a9e4590c..548e1bc20dace7 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2056,8 +2056,15 @@ make_pre_finalization_calls(PyThreadState *tstate) _PyAtExit_Call(tstate->interp); + /* Stop the world to prevent other threads from creating threads or + * atexit callbacks. On the default build, this is simply locked by + * the GIL. For pending calls, we acquire the dedicated mutex, because + * Py_AddPendingCall() can be called without an attached thread state. + */ + // XXX Why does _PyThreadState_DeleteList() rely on all interpreters // being stopped? + PyMutex_Lock(&interp->ceval.pending.mutex); _PyEval_StopTheWorldAll(interp->runtime); int should_continue = (interp_has_threads(interp) || interp_has_atexit_callbacks(interp) @@ -2066,6 +2073,7 @@ make_pre_finalization_calls(PyThreadState *tstate) break; } _PyEval_StartTheWorldAll(interp->runtime); + PyMutex_Unlock(&interp->ceval.pending.mutex); } assert(interp->runtime->stoptheworld.world_stopped); } @@ -2125,6 +2133,7 @@ _Py_Finalize(_PyRuntimeState *runtime) _PyThreadState_SetShuttingDown(p); } _PyEval_StartTheWorldAll(runtime); + PyMutex_Unlock(&tstate->interp->ceval.pending.mutex); /* Clear frames of other threads to call objects destructors. Destructors will be called in the current Python thread. Since @@ -2496,6 +2505,7 @@ Py_EndInterpreter(PyThreadState *tstate) when they attempt to take the GIL (ex: PyEval_RestoreThread()). */ _PyInterpreterState_SetFinalizing(interp, tstate); _PyEval_StartTheWorldAll(interp->runtime); + PyMutex_Unlock(&interp->ceval.pending.mutex); // XXX Call something like _PyImport_Disable() here? From 1e1301ddf563c7d185acf0f9b71a9ae0bf6c3db4 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Thu, 10 Jul 2025 09:59:14 -0400 Subject: [PATCH 21/21] Only check for non-daemon threads at finalization. Since threading._shutdown() only shuts down non-daemon threads, daemon threads would cause the loop to run indefinitely. --- Include/cpython/pystate.h | 1 + Modules/_threadmodule.c | 7 ++++--- Python/pylifecycle.c | 20 +++++++++++++++++++- Python/pystate.c | 2 +- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index be582122118e44..a55bc94c840d01 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -107,6 +107,7 @@ struct _ts { # define _PyThreadState_WHENCE_THREADING 3 # define _PyThreadState_WHENCE_GILSTATE 4 # define _PyThreadState_WHENCE_EXEC 5 +# define _PyThreadState_WHENCE_THREADING_DAEMON 6 #endif /* Currently holds the GIL. Must be its own field to avoid data races */ diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 8886a9d6bd0c8d..cdd797e8c61338 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -415,7 +415,7 @@ force_done(void *arg) static int ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args, - PyObject *kwargs) + PyObject *kwargs, int daemon) { // Mark the handle as starting to prevent any other threads from doing so PyMutex_Lock(&self->mutex); @@ -439,7 +439,8 @@ ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args, goto start_failed; } PyInterpreterState *interp = _PyInterpreterState_GET(); - boot->tstate = _PyThreadState_New(interp, _PyThreadState_WHENCE_THREADING); + uint8_t whence = daemon ? _PyThreadState_WHENCE_THREADING_DAEMON : _PyThreadState_WHENCE_THREADING; + boot->tstate = _PyThreadState_New(interp, whence); if (boot->tstate == NULL) { PyMem_RawFree(boot); if (!PyErr_Occurred()) { @@ -1874,7 +1875,7 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args, add_to_shutdown_handles(state, handle); } - if (ThreadHandle_start(handle, func, args, kwargs) < 0) { + if (ThreadHandle_start(handle, func, args, kwargs, daemon) < 0) { if (!daemon) { remove_from_shutdown_handles(handle); } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 548e1bc20dace7..d5b641e9ae9572 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2006,15 +2006,32 @@ resolve_final_tstate(_PyRuntimeState *runtime) static int interp_has_threads(PyInterpreterState *interp) { + /* This needs to check for non-daemon threads only, otherwise we get stuck + * in an infinite loop. */ assert(interp != NULL); + assert(interp->runtime->stoptheworld.world_stopped); assert(interp->threads.head != NULL); - return interp->threads.head->next != NULL; + if (interp->threads.head->next == NULL) { + // No other threads active, easy way out. + return 0; + } + + // We don't have to worry about locking this because the + // world is stopped. + _Py_FOR_EACH_TSTATE_UNLOCKED(interp, tstate) { + if (tstate->_whence == _PyThreadState_WHENCE_THREADING) { + return 1; + } + } + + return 0; } static int interp_has_pending_calls(PyInterpreterState *interp) { assert(interp != NULL); + assert(interp->runtime->stoptheworld.world_stopped); return interp->ceval.pending.npending != 0; } @@ -2023,6 +2040,7 @@ interp_has_atexit_callbacks(PyInterpreterState *interp) { assert(interp != NULL); assert(interp->atexit.callbacks != NULL); + assert(interp->runtime->stoptheworld.world_stopped); assert(PyList_CheckExact(interp->atexit.callbacks)); return PyList_GET_SIZE(interp->atexit.callbacks) != 0; } diff --git a/Python/pystate.c b/Python/pystate.c index 0d4c26f92cec90..32a80bb5212591 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1447,7 +1447,7 @@ init_threadstate(_PyThreadStateImpl *_tstate, assert(tstate->prev == NULL); assert(tstate->_whence == _PyThreadState_WHENCE_NOTSET); - assert(whence >= 0 && whence <= _PyThreadState_WHENCE_EXEC); + assert(whence >= 0 && whence <= _PyThreadState_WHENCE_THREADING_DAEMON); tstate->_whence = whence; assert(id > 0);