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/Lib/test/test_atexit.py b/Lib/test/test_atexit.py index eb01da6e88a8bc..66142a108d5d93 100644 --- a/Lib/test/test_atexit.py +++ b/Lib/test/test_atexit.py @@ -79,6 +79,62 @@ 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 = """if True: + 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() + """ + 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() + @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") + def test_thread_created_in_atexit_subinterpreter(self): + 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): diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index ef950f5df04ad3..918ccda18debe6 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, f"24{os.linesep}42{os.linesep}".encode("utf-8")) + self.assertEqual(stderr, b"") + class SubinterpreterTest(unittest.TestCase): diff --git a/Lib/threading.py b/Lib/threading.py index b6c451d1fbaabd..f28dea284a6b48 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 + # _shutdown() was already called, but threads might have started + # in the meantime. + return _thread_shutdown() global _SHUTTING_DOWN _SHUTTING_DOWN = True 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`). 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 */ }; 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 724fda63511282..d5b641e9ae9572 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2003,6 +2003,99 @@ resolve_final_tstate(_PyRuntimeState *runtime) return main_tstate; } +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); + 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; +} + +static int +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; +} + +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. */ + for (;;) { + assert(!interp->runtime->stoptheworld.world_stopped); + + // 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); + + /* 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) + || interp_has_pending_calls(interp)); + if (!should_continue) { + break; + } + _PyEval_StartTheWorldAll(interp->runtime); + PyMutex_Unlock(&interp->ceval.pending.mutex); + } + assert(interp->runtime->stoptheworld.world_stopped); +} + static int _Py_Finalize(_PyRuntimeState *runtime) { @@ -2019,23 +2112,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); @@ -2053,7 +2130,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()). */ @@ -2074,6 +2151,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 @@ -2435,13 +2513,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"); @@ -2450,6 +2522,8 @@ 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); + PyMutex_Unlock(&interp->ceval.pending.mutex); // XXX Call something like _PyImport_Disable() here? 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);