From 04c68030f169eff73bdb8f4674d4450903187e22 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 16 Sep 2024 15:35:29 -0700 Subject: [PATCH 01/13] gh-123940: Ensure force-terminated daemon threads can be joined During finalization, daemon threads are force to exit immediately (without returning through the call-stack normally) upon acquiring the GIL. Finalizers that run after this must be able to join the forcefully terminated threads. The current implementation notified of thread completion before returning from `thread_run`. This code will never execute if the thread is forced to exit during finalization. Any code that attempts to join such a thread will block indefinitely. To fix this, use the old approach of notifying of thread completion when the thread state is cleared. This happens both when `thread_run` exits normally and when thread states are destroyed as part of finalization (which happens immediately after forcing daemon threads to exit, before any python code can run). --- Include/cpython/pystate.h | 3 ++ Include/internal/pycore_lock.h | 10 +++++++ Include/internal/pycore_pystate.h | 3 ++ Lib/test/test_threading.py | 33 +++++++++++++++++++++ Modules/_threadmodule.c | 30 ++++++++++++------- Python/lock.c | 24 ++++++++++++++++ Python/pystate.c | 48 ++++++++++++++++++++++++++----- 7 files changed, 133 insertions(+), 18 deletions(-) diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index f005729fff11b6..34439505114543 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -200,6 +200,9 @@ struct _ts { The PyThreadObject must hold the only reference to this value. */ PyObject *threading_local_sentinel; + + /* Set when the thread is about to exit */ + struct _PyEventRc *thread_is_exiting; }; #ifdef Py_DEBUG diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index e6da083b807ce5..971f7a54a631e3 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -93,6 +93,16 @@ PyAPI_FUNC(void) PyEvent_Wait(PyEvent *evt); PyAPI_FUNC(int) PyEvent_WaitTimed(PyEvent *evt, PyTime_t timeout_ns, int detach); +// A one-time event notification with reference counting +typedef struct _PyEventRc { + PyEvent event; + Py_ssize_t refcount; +} _PyEventRc; + +extern _PyEventRc *_PyEventRc_New(void); +extern void _PyEventRc_Incref(_PyEventRc *erc); +extern void _PyEventRc_Decref(_PyEventRc *erc); + // _PyRawMutex implements a word-sized mutex that that does not depend on the // parking lot API, and therefore can be used in the parking lot // implementation. diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index fade55945b7dbf..d221cc1ddf0c8c 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -223,6 +223,9 @@ extern void _PyThreadState_Bind(PyThreadState *tstate); PyAPI_FUNC(PyThreadState *) _PyThreadState_NewBound( PyInterpreterState *interp, int whence); +extern PyThreadState * +_PyThreadState_NewWithEvent(PyInterpreterState *interp, int whence, + _PyEventRc *exiting_event); extern PyThreadState * _PyThreadState_RemoveExcept(PyThreadState *tstate); extern void _PyThreadState_DeleteList(PyThreadState *list); extern void _PyThreadState_ClearMimallocHeaps(PyThreadState *tstate); diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 329767aa82e336..4c5559d4fa40dc 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1171,6 +1171,39 @@ def __del__(self): self.assertEqual(out.strip(), b"OK") self.assertIn(b"can't create new thread at interpreter shutdown", err) + @unittest.skipIf(support.Py_GIL_DISABLED, "gh-124149: daemon threads don't force exit") + def test_join_force_terminated_daemon_thread_in_finalization(self): + # gh-123940: Py_Finalize() forces all daemon threads to exit + # immediately (without unwinding the stack) upon acquiring the + # GIL. Finalizers that run after this must be able to join the daemon + # threads that were forced to exit. + code = textwrap.dedent(""" + import threading + + + def loop(): + while True: + pass + + + class Cycle: + def __init__(self): + self.self_ref = self + self.thr = threading.Thread(target=loop, daemon=True) + self.thr.start() + + def __del__(self): + self.thr.join() + + # Cycle holds a reference to itself, which ensures it is cleaned + # up during the GC that runs after daemon threads have been + # forced to exit during finalization. + Cycle() + """) + rc, out, err = assert_python_ok("-c", code) + self.assertEqual(err, b"") + + class ThreadJoinOnShutdown(BaseTestCase): def _run_and_join(self, script): diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index b3ed8e7bc56b9e..eeca910f3658a1 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -96,7 +96,7 @@ typedef struct { // thread is about to exit. This is used to avoid false positives when // detecting self-join attempts. See the comment in `ThreadHandle_join()` // for a more detailed explanation. - PyEvent thread_is_exiting; + _PyEventRc *thread_is_exiting; // Serializes calls to `join` and `set_done`. _PyOnceFlag once; @@ -174,6 +174,12 @@ remove_from_shutdown_handles(ThreadHandle *handle) static ThreadHandle * ThreadHandle_new(void) { + _PyEventRc *exiting = _PyEventRc_New(); + if (exiting == NULL) { + PyErr_NoMemory(); + return NULL; + } + ThreadHandle *self = (ThreadHandle *)PyMem_RawCalloc(1, sizeof(ThreadHandle)); if (self == NULL) { @@ -183,7 +189,7 @@ ThreadHandle_new(void) self->ident = 0; self->os_handle = 0; self->has_os_handle = 0; - self->thread_is_exiting = (PyEvent){0}; + self->thread_is_exiting = exiting; self->mutex = (PyMutex){_Py_UNLOCKED}; self->once = (_PyOnceFlag){0}; self->state = THREAD_HANDLE_NOT_STARTED; @@ -226,6 +232,8 @@ ThreadHandle_decref(ThreadHandle *self) return; } + _PyEventRc_Decref(self->thread_is_exiting); + // Remove ourself from the global list of handles HEAD_LOCK(&_PyRuntime); if (self->node.next != NULL) { @@ -268,7 +276,7 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state) handle->state = THREAD_HANDLE_DONE; handle->once = (_PyOnceFlag){_Py_ONCE_INITIALIZED}; handle->mutex = (PyMutex){_Py_UNLOCKED}; - _PyEvent_Notify(&handle->thread_is_exiting); + _PyEvent_Notify(&handle->thread_is_exiting->event); llist_remove(node); remove_from_shutdown_handles(handle); } @@ -357,8 +365,6 @@ thread_run(void *boot_raw) exit: // Don't need to wait for this thread anymore remove_from_shutdown_handles(handle); - - _PyEvent_Notify(&handle->thread_is_exiting); ThreadHandle_decref(handle); // bpo-44434: Don't call explicitly PyThread_exit_thread(). On Linux with @@ -371,7 +377,7 @@ static int force_done(ThreadHandle *handle) { assert(get_thread_handle_state(handle) == THREAD_HANDLE_STARTING); - _PyEvent_Notify(&handle->thread_is_exiting); + _PyEvent_Notify(&handle->thread_is_exiting->event); set_thread_handle_state(handle, THREAD_HANDLE_DONE); return 0; } @@ -402,7 +408,8 @@ ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args, goto start_failed; } PyInterpreterState *interp = _PyInterpreterState_GET(); - boot->tstate = _PyThreadState_New(interp, _PyThreadState_WHENCE_THREADING); + boot->tstate = _PyThreadState_NewWithEvent( + interp, _PyThreadState_WHENCE_THREADING, self->thread_is_exiting); if (boot->tstate == NULL) { PyMem_RawFree(boot); if (!PyErr_Occurred()) { @@ -492,7 +499,7 @@ ThreadHandle_join(ThreadHandle *self, PyTime_t timeout_ns) // To work around this, we set `thread_is_exiting` immediately before // `thread_run` returns. We can be sure that we are not attempting to join // ourselves if the handle's thread is about to exit. - if (!_PyEvent_IsSet(&self->thread_is_exiting) && + if (!_PyEvent_IsSet(&self->thread_is_exiting->event) && ThreadHandle_ident(self) == PyThread_get_thread_ident_ex()) { // PyThread_join_thread() would deadlock or error out. PyErr_SetString(ThreadError, "Cannot join current thread"); @@ -502,7 +509,8 @@ ThreadHandle_join(ThreadHandle *self, PyTime_t timeout_ns) // Wait until the deadline for the thread to exit. PyTime_t deadline = timeout_ns != -1 ? _PyDeadline_Init(timeout_ns) : 0; int detach = 1; - while (!PyEvent_WaitTimed(&self->thread_is_exiting, timeout_ns, detach)) { + while (!PyEvent_WaitTimed(&self->thread_is_exiting->event, timeout_ns, + detach)) { if (deadline) { // _PyDeadline_Get will return a negative value if the deadline has // been exceeded. @@ -537,7 +545,7 @@ set_done(ThreadHandle *handle) PyErr_SetString(ThreadError, "failed detaching handle"); return -1; } - _PyEvent_Notify(&handle->thread_is_exiting); + _PyEvent_Notify(&handle->thread_is_exiting->event); set_thread_handle_state(handle, THREAD_HANDLE_DONE); return 0; } @@ -649,7 +657,7 @@ static PyObject * PyThreadHandleObject_is_done(PyThreadHandleObject *self, PyObject *Py_UNUSED(ignored)) { - if (_PyEvent_IsSet(&self->handle->thread_is_exiting)) { + if (_PyEvent_IsSet(&self->handle->thread_is_exiting->event)) { Py_RETURN_TRUE; } else { diff --git a/Python/lock.c b/Python/lock.c index 57675fe1873fa2..67e970f20824f8 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -294,6 +294,30 @@ PyEvent_WaitTimed(PyEvent *evt, PyTime_t timeout_ns, int detach) } } +_PyEventRc * +_PyEventRc_New(void) +{ + _PyEventRc *erc = (_PyEventRc *)PyMem_RawCalloc(1, sizeof(_PyEventRc)); + if (erc != NULL) { + erc->refcount = 1; + } + return erc; +} + +void +_PyEventRc_Incref(_PyEventRc *erc) +{ + _Py_atomic_add_ssize(&erc->refcount, 1); +} + +void +_PyEventRc_Decref(_PyEventRc *erc) +{ + if (_Py_atomic_add_ssize(&erc->refcount, -1) == 1) { + PyMem_RawFree(erc); + } +} + static int unlock_once(_PyOnceFlag *o, int res) { diff --git a/Python/pystate.c b/Python/pystate.c index 54caf373e91d6c..a4bb696d3f508d 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1458,8 +1458,8 @@ free_threadstate(_PyThreadStateImpl *tstate) */ static void -init_threadstate(_PyThreadStateImpl *_tstate, - PyInterpreterState *interp, uint64_t id, int whence) +init_threadstate(_PyThreadStateImpl *_tstate, PyInterpreterState *interp, + uint64_t id, int whence, _PyEventRc *exiting_event) { PyThreadState *tstate = (PyThreadState *)_tstate; if (tstate->_status.initialized) { @@ -1467,6 +1467,7 @@ init_threadstate(_PyThreadStateImpl *_tstate, } assert(interp != NULL); + tstate->thread_is_exiting = exiting_event; tstate->interp = interp; tstate->eval_breaker = _Py_atomic_load_uintptr_relaxed(&interp->ceval.instrumentation_version); @@ -1530,8 +1531,19 @@ add_threadstate(PyInterpreterState *interp, PyThreadState *tstate, interp->threads.head = tstate; } +static _PyEventRc * +ensure_event(_PyEventRc *exiting_event) +{ + if (exiting_event != NULL) { + _PyEventRc_Incref(exiting_event); + return exiting_event; + } + return _PyEventRc_New(); +} + static PyThreadState * -new_threadstate(PyInterpreterState *interp, int whence) +new_threadstate(PyInterpreterState *interp, int whence, + _PyEventRc *exiting_event) { _PyThreadStateImpl *tstate; _PyRuntimeState *runtime = interp->runtime; @@ -1544,10 +1556,18 @@ new_threadstate(PyInterpreterState *interp, int whence) if (new_tstate == NULL) { return NULL; } + + exiting_event = ensure_event(exiting_event); + if (exiting_event == NULL) { + PyMem_RawFree(new_tstate); + return NULL; + } + #ifdef Py_GIL_DISABLED Py_ssize_t qsbr_idx = _Py_qsbr_reserve(interp); if (qsbr_idx < 0) { PyMem_RawFree(new_tstate); + _PyEventRc_Decref(exiting_event); return NULL; } #endif @@ -1578,7 +1598,7 @@ new_threadstate(PyInterpreterState *interp, int whence) sizeof(*tstate)); } - init_threadstate(tstate, interp, id, whence); + init_threadstate(tstate, interp, id, whence, exiting_event); add_threadstate(interp, (PyThreadState *)tstate, old_head); HEAD_UNLOCK(runtime); @@ -1606,7 +1626,7 @@ PyThreadState_New(PyInterpreterState *interp) PyThreadState * _PyThreadState_NewBound(PyInterpreterState *interp, int whence) { - PyThreadState *tstate = new_threadstate(interp, whence); + PyThreadState *tstate = new_threadstate(interp, whence, NULL); if (tstate) { bind_tstate(tstate); // This makes sure there's a gilstate tstate bound @@ -1622,7 +1642,14 @@ _PyThreadState_NewBound(PyInterpreterState *interp, int whence) PyThreadState * _PyThreadState_New(PyInterpreterState *interp, int whence) { - return new_threadstate(interp, whence); + return new_threadstate(interp, whence, NULL); +} + +PyThreadState * +_PyThreadState_NewWithEvent(PyInterpreterState *interp, int whence, + _PyEventRc *exiting_event) +{ + return new_threadstate(interp, whence, exiting_event); } // We keep this for stable ABI compabibility. @@ -1732,6 +1759,13 @@ PyThreadState_Clear(PyThreadState *tstate) Py_CLEAR(tstate->context); + if (tstate->thread_is_exiting != NULL) { + _PyEventRc *erc = tstate->thread_is_exiting; + tstate->thread_is_exiting = NULL; + _PyEvent_Notify(&erc->event); + _PyEventRc_Decref(erc); + } + #ifdef Py_GIL_DISABLED // Each thread should clear own freelists in free-threading builds. struct _Py_freelists *freelists = _Py_freelists_GET(); @@ -2760,7 +2794,7 @@ PyGILState_Ensure(void) /* Create a new Python thread state for this thread */ // XXX Use PyInterpreterState_EnsureThreadState()? tcur = new_threadstate(runtime->gilstate.autoInterpreterState, - _PyThreadState_WHENCE_GILSTATE); + _PyThreadState_WHENCE_GILSTATE, NULL); if (tcur == NULL) { Py_FatalError("Couldn't create thread-state for new thread"); } From 679ce7d13b22ccc409c939499a32dece7eb7b7a6 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 16 Sep 2024 18:42:44 -0700 Subject: [PATCH 02/13] Add suppression for _PyThreadState_MustExit --- Tools/tsan/suppressions_free_threading.txt | 4 ++++ Tools/tsan/supressions.txt | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt index e5eb665ae212de..19808ada1bca2a 100644 --- a/Tools/tsan/suppressions_free_threading.txt +++ b/Tools/tsan/suppressions_free_threading.txt @@ -9,6 +9,10 @@ race:get_allocator_unlocked race:set_allocator_unlocked +# https://gist.github.com/mpage/ff1c7094bc8237d98387678d5f52058f +race_top:_PyThreadState_MustExit + + ## Free-threaded suppressions diff --git a/Tools/tsan/supressions.txt b/Tools/tsan/supressions.txt index 22ba9d6ba2ab4d..f9841d4da24a6e 100644 --- a/Tools/tsan/supressions.txt +++ b/Tools/tsan/supressions.txt @@ -3,5 +3,9 @@ race:get_allocator_unlocked race:set_allocator_unlocked + +# https://gist.github.com/mpage/ff1c7094bc8237d98387678d5f52058f +race_top:_PyThreadState_MustExit + # https://gist.github.com/mpage/daaf32b39180c1989572957b943eb665 thread:pthread_create From cd7b7d1082e3656f68401b414f4da41b23667af1 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Mon, 16 Sep 2024 19:36:13 -0700 Subject: [PATCH 03/13] Remove incorrect warnignore line --- Tools/build/.warningignore_ubuntu | 1 - 1 file changed, 1 deletion(-) diff --git a/Tools/build/.warningignore_ubuntu b/Tools/build/.warningignore_ubuntu index e98305e81808d6..d170f727946fbc 100644 --- a/Tools/build/.warningignore_ubuntu +++ b/Tools/build/.warningignore_ubuntu @@ -233,7 +233,6 @@ Python/generated_cases.c.h 27 Python/generated_cases.c.h 27 Python/getargs.c 7 Python/hashtable.c 1 -Python/import.c 6 Python/import.c 7 Python/initconfig.c 11 Python/instrumentation.c 43 From bf186e2f469c6cc9f61755ce4411e8c9dcbe8241 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 17 Sep 2024 15:42:20 -0700 Subject: [PATCH 04/13] Assert that finalizer is called --- Lib/test/test_threading.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 4c5559d4fa40dc..d32951a9ae55c0 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1194,6 +1194,7 @@ def __init__(self): def __del__(self): self.thr.join() + print('__del__ called') # Cycle holds a reference to itself, which ensures it is cleaned # up during the GC that runs after daemon threads have been @@ -1202,6 +1203,7 @@ def __del__(self): """) rc, out, err = assert_python_ok("-c", code) self.assertEqual(err, b"") + self.assertIn(b"__del__ called", out) class ThreadJoinOnShutdown(BaseTestCase): From 96613eda3f5d6dbd9ee54c7caef90e0d15bb627b Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 17 Sep 2024 15:44:18 -0700 Subject: [PATCH 05/13] Note the test that triggers the race in _PyThreadState_MustExit --- Tools/tsan/suppressions_free_threading.txt | 1 + Tools/tsan/supressions.txt | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt index 19808ada1bca2a..b99597c0421651 100644 --- a/Tools/tsan/suppressions_free_threading.txt +++ b/Tools/tsan/suppressions_free_threading.txt @@ -9,6 +9,7 @@ race:get_allocator_unlocked race:set_allocator_unlocked +# Triggered by test_threading.ThreadTests.test_join_force_terminated_daemon_thread_in_finalization # https://gist.github.com/mpage/ff1c7094bc8237d98387678d5f52058f race_top:_PyThreadState_MustExit diff --git a/Tools/tsan/supressions.txt b/Tools/tsan/supressions.txt index f9841d4da24a6e..c20ad26e95077e 100644 --- a/Tools/tsan/supressions.txt +++ b/Tools/tsan/supressions.txt @@ -3,7 +3,7 @@ race:get_allocator_unlocked race:set_allocator_unlocked - +# Triggered by test_threading.ThreadTests.test_join_force_terminated_daemon_thread_in_finalization # https://gist.github.com/mpage/ff1c7094bc8237d98387678d5f52058f race_top:_PyThreadState_MustExit From 33ab6542b390d58636e836e2db6a92dd54f46273 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 17 Sep 2024 15:48:41 -0700 Subject: [PATCH 06/13] Revert changes --- Include/cpython/pystate.h | 3 -- Include/internal/pycore_lock.h | 10 ------- Include/internal/pycore_pystate.h | 3 -- Modules/_threadmodule.c | 30 +++++++------------ Python/lock.c | 24 ---------------- Python/pystate.c | 48 +++++-------------------------- 6 files changed, 18 insertions(+), 100 deletions(-) diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index 34439505114543..f005729fff11b6 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -200,9 +200,6 @@ struct _ts { The PyThreadObject must hold the only reference to this value. */ PyObject *threading_local_sentinel; - - /* Set when the thread is about to exit */ - struct _PyEventRc *thread_is_exiting; }; #ifdef Py_DEBUG diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index 971f7a54a631e3..e6da083b807ce5 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -93,16 +93,6 @@ PyAPI_FUNC(void) PyEvent_Wait(PyEvent *evt); PyAPI_FUNC(int) PyEvent_WaitTimed(PyEvent *evt, PyTime_t timeout_ns, int detach); -// A one-time event notification with reference counting -typedef struct _PyEventRc { - PyEvent event; - Py_ssize_t refcount; -} _PyEventRc; - -extern _PyEventRc *_PyEventRc_New(void); -extern void _PyEventRc_Incref(_PyEventRc *erc); -extern void _PyEventRc_Decref(_PyEventRc *erc); - // _PyRawMutex implements a word-sized mutex that that does not depend on the // parking lot API, and therefore can be used in the parking lot // implementation. diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index d221cc1ddf0c8c..fade55945b7dbf 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -223,9 +223,6 @@ extern void _PyThreadState_Bind(PyThreadState *tstate); PyAPI_FUNC(PyThreadState *) _PyThreadState_NewBound( PyInterpreterState *interp, int whence); -extern PyThreadState * -_PyThreadState_NewWithEvent(PyInterpreterState *interp, int whence, - _PyEventRc *exiting_event); extern PyThreadState * _PyThreadState_RemoveExcept(PyThreadState *tstate); extern void _PyThreadState_DeleteList(PyThreadState *list); extern void _PyThreadState_ClearMimallocHeaps(PyThreadState *tstate); diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index eeca910f3658a1..b3ed8e7bc56b9e 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -96,7 +96,7 @@ typedef struct { // thread is about to exit. This is used to avoid false positives when // detecting self-join attempts. See the comment in `ThreadHandle_join()` // for a more detailed explanation. - _PyEventRc *thread_is_exiting; + PyEvent thread_is_exiting; // Serializes calls to `join` and `set_done`. _PyOnceFlag once; @@ -174,12 +174,6 @@ remove_from_shutdown_handles(ThreadHandle *handle) static ThreadHandle * ThreadHandle_new(void) { - _PyEventRc *exiting = _PyEventRc_New(); - if (exiting == NULL) { - PyErr_NoMemory(); - return NULL; - } - ThreadHandle *self = (ThreadHandle *)PyMem_RawCalloc(1, sizeof(ThreadHandle)); if (self == NULL) { @@ -189,7 +183,7 @@ ThreadHandle_new(void) self->ident = 0; self->os_handle = 0; self->has_os_handle = 0; - self->thread_is_exiting = exiting; + self->thread_is_exiting = (PyEvent){0}; self->mutex = (PyMutex){_Py_UNLOCKED}; self->once = (_PyOnceFlag){0}; self->state = THREAD_HANDLE_NOT_STARTED; @@ -232,8 +226,6 @@ ThreadHandle_decref(ThreadHandle *self) return; } - _PyEventRc_Decref(self->thread_is_exiting); - // Remove ourself from the global list of handles HEAD_LOCK(&_PyRuntime); if (self->node.next != NULL) { @@ -276,7 +268,7 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state) handle->state = THREAD_HANDLE_DONE; handle->once = (_PyOnceFlag){_Py_ONCE_INITIALIZED}; handle->mutex = (PyMutex){_Py_UNLOCKED}; - _PyEvent_Notify(&handle->thread_is_exiting->event); + _PyEvent_Notify(&handle->thread_is_exiting); llist_remove(node); remove_from_shutdown_handles(handle); } @@ -365,6 +357,8 @@ thread_run(void *boot_raw) exit: // Don't need to wait for this thread anymore remove_from_shutdown_handles(handle); + + _PyEvent_Notify(&handle->thread_is_exiting); ThreadHandle_decref(handle); // bpo-44434: Don't call explicitly PyThread_exit_thread(). On Linux with @@ -377,7 +371,7 @@ static int force_done(ThreadHandle *handle) { assert(get_thread_handle_state(handle) == THREAD_HANDLE_STARTING); - _PyEvent_Notify(&handle->thread_is_exiting->event); + _PyEvent_Notify(&handle->thread_is_exiting); set_thread_handle_state(handle, THREAD_HANDLE_DONE); return 0; } @@ -408,8 +402,7 @@ ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args, goto start_failed; } PyInterpreterState *interp = _PyInterpreterState_GET(); - boot->tstate = _PyThreadState_NewWithEvent( - interp, _PyThreadState_WHENCE_THREADING, self->thread_is_exiting); + boot->tstate = _PyThreadState_New(interp, _PyThreadState_WHENCE_THREADING); if (boot->tstate == NULL) { PyMem_RawFree(boot); if (!PyErr_Occurred()) { @@ -499,7 +492,7 @@ ThreadHandle_join(ThreadHandle *self, PyTime_t timeout_ns) // To work around this, we set `thread_is_exiting` immediately before // `thread_run` returns. We can be sure that we are not attempting to join // ourselves if the handle's thread is about to exit. - if (!_PyEvent_IsSet(&self->thread_is_exiting->event) && + if (!_PyEvent_IsSet(&self->thread_is_exiting) && ThreadHandle_ident(self) == PyThread_get_thread_ident_ex()) { // PyThread_join_thread() would deadlock or error out. PyErr_SetString(ThreadError, "Cannot join current thread"); @@ -509,8 +502,7 @@ ThreadHandle_join(ThreadHandle *self, PyTime_t timeout_ns) // Wait until the deadline for the thread to exit. PyTime_t deadline = timeout_ns != -1 ? _PyDeadline_Init(timeout_ns) : 0; int detach = 1; - while (!PyEvent_WaitTimed(&self->thread_is_exiting->event, timeout_ns, - detach)) { + while (!PyEvent_WaitTimed(&self->thread_is_exiting, timeout_ns, detach)) { if (deadline) { // _PyDeadline_Get will return a negative value if the deadline has // been exceeded. @@ -545,7 +537,7 @@ set_done(ThreadHandle *handle) PyErr_SetString(ThreadError, "failed detaching handle"); return -1; } - _PyEvent_Notify(&handle->thread_is_exiting->event); + _PyEvent_Notify(&handle->thread_is_exiting); set_thread_handle_state(handle, THREAD_HANDLE_DONE); return 0; } @@ -657,7 +649,7 @@ static PyObject * PyThreadHandleObject_is_done(PyThreadHandleObject *self, PyObject *Py_UNUSED(ignored)) { - if (_PyEvent_IsSet(&self->handle->thread_is_exiting->event)) { + if (_PyEvent_IsSet(&self->handle->thread_is_exiting)) { Py_RETURN_TRUE; } else { diff --git a/Python/lock.c b/Python/lock.c index 67e970f20824f8..57675fe1873fa2 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -294,30 +294,6 @@ PyEvent_WaitTimed(PyEvent *evt, PyTime_t timeout_ns, int detach) } } -_PyEventRc * -_PyEventRc_New(void) -{ - _PyEventRc *erc = (_PyEventRc *)PyMem_RawCalloc(1, sizeof(_PyEventRc)); - if (erc != NULL) { - erc->refcount = 1; - } - return erc; -} - -void -_PyEventRc_Incref(_PyEventRc *erc) -{ - _Py_atomic_add_ssize(&erc->refcount, 1); -} - -void -_PyEventRc_Decref(_PyEventRc *erc) -{ - if (_Py_atomic_add_ssize(&erc->refcount, -1) == 1) { - PyMem_RawFree(erc); - } -} - static int unlock_once(_PyOnceFlag *o, int res) { diff --git a/Python/pystate.c b/Python/pystate.c index a4bb696d3f508d..54caf373e91d6c 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1458,8 +1458,8 @@ free_threadstate(_PyThreadStateImpl *tstate) */ static void -init_threadstate(_PyThreadStateImpl *_tstate, PyInterpreterState *interp, - uint64_t id, int whence, _PyEventRc *exiting_event) +init_threadstate(_PyThreadStateImpl *_tstate, + PyInterpreterState *interp, uint64_t id, int whence) { PyThreadState *tstate = (PyThreadState *)_tstate; if (tstate->_status.initialized) { @@ -1467,7 +1467,6 @@ init_threadstate(_PyThreadStateImpl *_tstate, PyInterpreterState *interp, } assert(interp != NULL); - tstate->thread_is_exiting = exiting_event; tstate->interp = interp; tstate->eval_breaker = _Py_atomic_load_uintptr_relaxed(&interp->ceval.instrumentation_version); @@ -1531,19 +1530,8 @@ add_threadstate(PyInterpreterState *interp, PyThreadState *tstate, interp->threads.head = tstate; } -static _PyEventRc * -ensure_event(_PyEventRc *exiting_event) -{ - if (exiting_event != NULL) { - _PyEventRc_Incref(exiting_event); - return exiting_event; - } - return _PyEventRc_New(); -} - static PyThreadState * -new_threadstate(PyInterpreterState *interp, int whence, - _PyEventRc *exiting_event) +new_threadstate(PyInterpreterState *interp, int whence) { _PyThreadStateImpl *tstate; _PyRuntimeState *runtime = interp->runtime; @@ -1556,18 +1544,10 @@ new_threadstate(PyInterpreterState *interp, int whence, if (new_tstate == NULL) { return NULL; } - - exiting_event = ensure_event(exiting_event); - if (exiting_event == NULL) { - PyMem_RawFree(new_tstate); - return NULL; - } - #ifdef Py_GIL_DISABLED Py_ssize_t qsbr_idx = _Py_qsbr_reserve(interp); if (qsbr_idx < 0) { PyMem_RawFree(new_tstate); - _PyEventRc_Decref(exiting_event); return NULL; } #endif @@ -1598,7 +1578,7 @@ new_threadstate(PyInterpreterState *interp, int whence, sizeof(*tstate)); } - init_threadstate(tstate, interp, id, whence, exiting_event); + init_threadstate(tstate, interp, id, whence); add_threadstate(interp, (PyThreadState *)tstate, old_head); HEAD_UNLOCK(runtime); @@ -1626,7 +1606,7 @@ PyThreadState_New(PyInterpreterState *interp) PyThreadState * _PyThreadState_NewBound(PyInterpreterState *interp, int whence) { - PyThreadState *tstate = new_threadstate(interp, whence, NULL); + PyThreadState *tstate = new_threadstate(interp, whence); if (tstate) { bind_tstate(tstate); // This makes sure there's a gilstate tstate bound @@ -1642,14 +1622,7 @@ _PyThreadState_NewBound(PyInterpreterState *interp, int whence) PyThreadState * _PyThreadState_New(PyInterpreterState *interp, int whence) { - return new_threadstate(interp, whence, NULL); -} - -PyThreadState * -_PyThreadState_NewWithEvent(PyInterpreterState *interp, int whence, - _PyEventRc *exiting_event) -{ - return new_threadstate(interp, whence, exiting_event); + return new_threadstate(interp, whence); } // We keep this for stable ABI compabibility. @@ -1759,13 +1732,6 @@ PyThreadState_Clear(PyThreadState *tstate) Py_CLEAR(tstate->context); - if (tstate->thread_is_exiting != NULL) { - _PyEventRc *erc = tstate->thread_is_exiting; - tstate->thread_is_exiting = NULL; - _PyEvent_Notify(&erc->event); - _PyEventRc_Decref(erc); - } - #ifdef Py_GIL_DISABLED // Each thread should clear own freelists in free-threading builds. struct _Py_freelists *freelists = _Py_freelists_GET(); @@ -2794,7 +2760,7 @@ PyGILState_Ensure(void) /* Create a new Python thread state for this thread */ // XXX Use PyInterpreterState_EnsureThreadState()? tcur = new_threadstate(runtime->gilstate.autoInterpreterState, - _PyThreadState_WHENCE_GILSTATE, NULL); + _PyThreadState_WHENCE_GILSTATE); if (tcur == NULL) { Py_FatalError("Couldn't create thread-state for new thread"); } From 192b260c7368ed9f2aa5fc1b9032e0f6690dc598 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Tue, 17 Sep 2024 18:46:49 -0700 Subject: [PATCH 07/13] Mark all daemon threads as done during finalization --- .../pycore_global_objects_fini_generated.h | 1 + Include/internal/pycore_global_strings.h | 1 + .../internal/pycore_runtime_init_generated.h | 1 + .../internal/pycore_unicodeobject_generated.h | 4 + Lib/test/test_threading.py | 1 - Lib/threading.py | 1 + Modules/_threadmodule.c | 80 +++++++++++++++---- Python/pylifecycle.c | 31 +++++-- 8 files changed, 96 insertions(+), 24 deletions(-) diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index 6e948e16b7dbe8..0c8319520c5d21 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -745,6 +745,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_blksize)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_bootstrap)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_check_retval_)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_daemon_threads_exited)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_dealloc_warn)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_feature_version)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_field_types)); diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index 5c63a6e519b93d..acd3a9b7043427 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -234,6 +234,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(_blksize) STRUCT_FOR_ID(_bootstrap) STRUCT_FOR_ID(_check_retval_) + STRUCT_FOR_ID(_daemon_threads_exited) STRUCT_FOR_ID(_dealloc_warn) STRUCT_FOR_ID(_feature_version) STRUCT_FOR_ID(_field_types) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index bac6b5b8fcfd9d..57d6eef73dbcf5 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -743,6 +743,7 @@ extern "C" { INIT_ID(_blksize), \ INIT_ID(_bootstrap), \ INIT_ID(_check_retval_), \ + INIT_ID(_daemon_threads_exited), \ INIT_ID(_dealloc_warn), \ INIT_ID(_feature_version), \ INIT_ID(_field_types), \ diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h index efdbde4c8ea3c6..c4ef3bbbbee3cf 100644 --- a/Include/internal/pycore_unicodeobject_generated.h +++ b/Include/internal/pycore_unicodeobject_generated.h @@ -736,6 +736,10 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); assert(PyUnicode_GET_LENGTH(string) != 1); + string = &_Py_ID(_daemon_threads_exited); + _PyUnicode_InternStatic(interp, &string); + assert(_PyUnicode_CheckConsistency(string, 1)); + assert(PyUnicode_GET_LENGTH(string) != 1); string = &_Py_ID(_dealloc_warn); _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index d32951a9ae55c0..a2db6cb5829942 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1171,7 +1171,6 @@ def __del__(self): self.assertEqual(out.strip(), b"OK") self.assertIn(b"can't create new thread at interpreter shutdown", err) - @unittest.skipIf(support.Py_GIL_DISABLED, "gh-124149: daemon threads don't force exit") def test_join_force_terminated_daemon_thread_in_finalization(self): # gh-123940: Py_Finalize() forces all daemon threads to exit # immediately (without unwinding the stack) upon acquiring the diff --git a/Lib/threading.py b/Lib/threading.py index 94ea2f08178369..14ea29c0cfab1c 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -42,6 +42,7 @@ get_ident = _thread.get_ident _get_main_thread_ident = _thread._get_main_thread_ident _is_main_interpreter = _thread._is_main_interpreter +_daemon_threads_exited = _thread._daemon_threads_exited try: get_native_id = _thread.get_native_id _HAVE_THREAD_NATIVE_ID = True diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index b3ed8e7bc56b9e..54d030adc4cda4 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -33,7 +33,11 @@ typedef struct { // Linked list of handles to all non-daemon threads created by the // threading module. We wait for these to finish at shutdown. - struct llist_node shutdown_handles; + struct llist_node non_daemon_handles; + + // Linked list of handles to daemon threads created by the threading + // module. We set the events in these handles as done at shutdown. + struct llist_node daemon_handles; } thread_module_state; static inline thread_module_state* @@ -78,7 +82,8 @@ typedef enum { typedef struct { struct llist_node node; // linked list node (see _pythread_runtime_state) - // linked list node (see thread_module_state) + // belongs to either `non_daemon_handles` or `daemon_handles` (see + // thread_module_state) struct llist_node shutdown_node; // The `ident`, `os_handle`, `has_os_handle`, and `state` fields are @@ -143,10 +148,11 @@ 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(struct llist_node *shutdown_handles, + ThreadHandle *handle) { HEAD_LOCK(&_PyRuntime); - llist_insert_tail(&state->shutdown_handles, &handle->shutdown_node); + llist_insert_tail(shutdown_handles, &handle->shutdown_node); HEAD_UNLOCK(&_PyRuntime); } @@ -155,7 +161,10 @@ clear_shutdown_handles(thread_module_state *state) { HEAD_LOCK(&_PyRuntime); struct llist_node *node; - llist_for_each_safe(node, &state->shutdown_handles) { + llist_for_each_safe(node, &state->daemon_handles) { + llist_remove(node); + } + llist_for_each_safe(node, &state->non_daemon_handles) { llist_remove(node); } HEAD_UNLOCK(&_PyRuntime); @@ -378,7 +387,7 @@ force_done(ThreadHandle *handle) static int ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args, - PyObject *kwargs) + PyObject *kwargs, struct llist_node *shutdown_handles) { // Mark the handle as starting to prevent any other threads from doing so PyMutex_Lock(&self->mutex); @@ -390,6 +399,11 @@ ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args, self->state = THREAD_HANDLE_STARTING; PyMutex_Unlock(&self->mutex); + // 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(shutdown_handles, self); + // Do all the heavy lifting outside of the mutex. All other operations on // the handle should fail since the handle is in the starting state. @@ -441,6 +455,7 @@ ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args, return 0; start_failed: + remove_from_shutdown_handles(self); _PyOnceFlag_CallOnce(&self->once, (_Py_once_fn_t *)force_done, self); return -1; } @@ -1872,17 +1887,12 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args, return -1; } - if (!daemon) { - // 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); + struct llist_node *shutdown_handles = &state->non_daemon_handles; + if (daemon) { + shutdown_handles = &state->daemon_handles; } - if (ThreadHandle_start(handle, func, args, kwargs) < 0) { - if (!daemon) { - remove_from_shutdown_handles(handle); - } + if (ThreadHandle_start(handle, func, args, kwargs, shutdown_handles) < 0) { return -1; } @@ -2372,7 +2382,7 @@ thread_shutdown(PyObject *self, PyObject *args) // Find a thread that's not yet finished. HEAD_LOCK(&_PyRuntime); struct llist_node *node; - llist_for_each_safe(node, &state->shutdown_handles) { + llist_for_each_safe(node, &state->non_daemon_handles) { ThreadHandle *cur = llist_data(node, ThreadHandle, shutdown_node); if (cur->ident != ident) { ThreadHandle_incref(cur); @@ -2407,6 +2417,39 @@ PyDoc_STRVAR(shutdown_doc, \n\ Wait for all non-daemon threads (other than the calling thread) to stop."); +static PyObject * +thread__daemon_threads_exited(PyObject *self, PyObject *args) +{ + thread_module_state *state = get_thread_state(self); + + for (;;) { + HEAD_LOCK(&_PyRuntime); + if (llist_empty(&state->daemon_handles)) { + HEAD_UNLOCK(&_PyRuntime); + break; + } + struct llist_node *node = state->daemon_handles.next; + ThreadHandle *handle = llist_data(node, ThreadHandle, shutdown_node); + ThreadHandle_incref(handle); + llist_remove(node); + HEAD_UNLOCK(&_PyRuntime); + // gh-123940: Mark daemon threads as done so that they can be joined + // from finalizers. + if (ThreadHandle_set_done(handle) < 0) { + PyErr_WriteUnraisable(NULL); + } + ThreadHandle_decref(handle); + } + + Py_RETURN_NONE; +} + +PyDoc_STRVAR(_daemon_threads_exited_doc, +"_daemon_threads_exited($module, /)\n\ +--\n\ +\n\ +Mark that daemon threads have exited."); + static PyObject * thread__make_thread_handle(PyObject *module, PyObject *identobj) { @@ -2486,6 +2529,8 @@ static PyMethodDef thread_methods[] = { METH_NOARGS, thread__is_main_interpreter_doc}, {"_shutdown", thread_shutdown, METH_NOARGS, shutdown_doc}, + {"_daemon_threads_exited", thread__daemon_threads_exited, + METH_NOARGS, _daemon_threads_exited_doc}, {"_make_thread_handle", thread__make_thread_handle, METH_O, thread__make_thread_handle_doc}, {"_get_main_thread_ident", thread__get_main_thread_ident, @@ -2579,7 +2624,8 @@ thread_module_exec(PyObject *module) return -1; } - llist_init(&state->shutdown_handles); + llist_init(&state->daemon_handles); + llist_init(&state->non_daemon_handles); return 0; } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 27faf723745c21..5737f7c8b76528 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -74,7 +74,8 @@ static PyStatus init_sys_streams(PyThreadState *tstate); #ifdef __ANDROID__ static PyStatus init_android_streams(PyThreadState *tstate); #endif -static void wait_for_thread_shutdown(PyThreadState *tstate); +static PyObject *wait_for_thread_shutdown(PyThreadState *tstate); +static void daemon_threads_exited(PyObject *threading_module); static void finalize_subinterpreters(void); static void call_ll_exitfuncs(_PyRuntimeState *runtime); @@ -1985,7 +1986,7 @@ _Py_Finalize(_PyRuntimeState *runtime) tstate->interp->finalizing = 1; // Wrap up existing "threading"-module-created, non-daemon threads. - wait_for_thread_shutdown(tstate); + PyObject *threading_module = wait_for_thread_shutdown(tstate); // Make any remaining pending calls. _Py_FinishPendingCalls(tstate); @@ -2040,6 +2041,7 @@ _Py_Finalize(_PyRuntimeState *runtime) before we call destructors. */ PyThreadState *list = _PyThreadState_RemoveExcept(tstate); _PyEval_StartTheWorldAll(runtime); + daemon_threads_exited(threading_module); _PyThreadState_DeleteList(list); /* At this point no Python code should be running at all. @@ -2388,7 +2390,7 @@ Py_EndInterpreter(PyThreadState *tstate) interp->finalizing = 1; // Wrap up existing "threading"-module-created, non-daemon threads. - wait_for_thread_shutdown(tstate); + PyObject *threading_module = wait_for_thread_shutdown(tstate); // Make any remaining pending calls. _Py_FinishPendingCalls(tstate); @@ -2403,6 +2405,7 @@ Py_EndInterpreter(PyThreadState *tstate) when they attempt to take the GIL (ex: PyEval_RestoreThread()). */ _PyInterpreterState_SetFinalizing(interp, tstate); + daemon_threads_exited(threading_module); // XXX Call something like _PyImport_Disable() here? _PyImport_FiniExternal(tstate->interp); @@ -3328,7 +3331,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 PyObject * wait_for_thread_shutdown(PyThreadState *tstate) { PyObject *result; @@ -3338,7 +3341,7 @@ wait_for_thread_shutdown(PyThreadState *tstate) PyErr_FormatUnraisable("Exception ignored on threading shutdown"); } /* else: threading not imported */ - return; + return NULL; } result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown)); if (result == NULL) { @@ -3347,7 +3350,23 @@ wait_for_thread_shutdown(PyThreadState *tstate) else { Py_DECREF(result); } - Py_DECREF(threading); + return threading; +} + +static void +daemon_threads_exited(PyObject *threading_module) +{ + if (threading_module == NULL) { + return; + } + PyObject *result = PyObject_CallMethodNoArgs( + threading_module, &_Py_ID(_daemon_threads_exited)); + if (result == NULL) { + PyErr_FormatUnraisable( + "Exception ignored while marking daemon threads exited"); + } + Py_XDECREF(result); + Py_DECREF(threading_module); } int Py_AtExit(void (*func)(void)) From aa1039a200d9ba7f963171771db9dd96bcd7700d Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 18 Sep 2024 10:38:44 -0700 Subject: [PATCH 08/13] Only notify, don't mark handles as done --- Lib/test/test_threading.py | 1 + Modules/_threadmodule.c | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index a2db6cb5829942..d32951a9ae55c0 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -1171,6 +1171,7 @@ def __del__(self): self.assertEqual(out.strip(), b"OK") self.assertIn(b"can't create new thread at interpreter shutdown", err) + @unittest.skipIf(support.Py_GIL_DISABLED, "gh-124149: daemon threads don't force exit") def test_join_force_terminated_daemon_thread_in_finalization(self): # gh-123940: Py_Finalize() forces all daemon threads to exit # immediately (without unwinding the stack) upon acquiring the diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 54d030adc4cda4..5903fbc1466350 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -2435,9 +2435,7 @@ thread__daemon_threads_exited(PyObject *self, PyObject *args) HEAD_UNLOCK(&_PyRuntime); // gh-123940: Mark daemon threads as done so that they can be joined // from finalizers. - if (ThreadHandle_set_done(handle) < 0) { - PyErr_WriteUnraisable(NULL); - } + _PyEvent_Notify(&handle->thread_is_exiting); ThreadHandle_decref(handle); } From 3978cc0625d8a0c312634c7aa734da91b6f79e59 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 18 Sep 2024 10:44:35 -0700 Subject: [PATCH 09/13] Document contract between wait_for_thread_shutdown and daemon_threads_exited --- Python/pylifecycle.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 5737f7c8b76528..755344cb638b3a 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -3330,7 +3330,12 @@ Py_ExitStatusException(PyStatus status) /* Wait until threading._shutdown completes, provided the threading module was imported in the first place. The shutdown routine will wait until all non-daemon - "threading" threads have completed. */ + "threading" threads have completed. + + Returns a reference to the threading module. `daemon_threads_exited` should + be called with this reference after the interpreter has been marked + finalizing. +*/ static PyObject * wait_for_thread_shutdown(PyThreadState *tstate) { @@ -3353,6 +3358,7 @@ wait_for_thread_shutdown(PyThreadState *tstate) return threading; } +/* Steals a reference to threading_module */ static void daemon_threads_exited(PyObject *threading_module) { From 5991fa4377535327d56f91dcd8177c89656819a2 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 18 Sep 2024 11:00:10 -0700 Subject: [PATCH 10/13] Don't pass threading module between wait_for_thread_shutdown and daemon_threads_exited --- Python/pylifecycle.c | 45 +++++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 755344cb638b3a..0bd6d8ba678f49 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -74,8 +74,8 @@ static PyStatus init_sys_streams(PyThreadState *tstate); #ifdef __ANDROID__ static PyStatus init_android_streams(PyThreadState *tstate); #endif -static PyObject *wait_for_thread_shutdown(PyThreadState *tstate); -static void daemon_threads_exited(PyObject *threading_module); +static void wait_for_thread_shutdown(PyThreadState *tstate); +static void daemon_threads_exited(PyThreadState *tstate); static void finalize_subinterpreters(void); static void call_ll_exitfuncs(_PyRuntimeState *runtime); @@ -1986,7 +1986,7 @@ _Py_Finalize(_PyRuntimeState *runtime) tstate->interp->finalizing = 1; // Wrap up existing "threading"-module-created, non-daemon threads. - PyObject *threading_module = wait_for_thread_shutdown(tstate); + wait_for_thread_shutdown(tstate); // Make any remaining pending calls. _Py_FinishPendingCalls(tstate); @@ -2041,7 +2041,7 @@ _Py_Finalize(_PyRuntimeState *runtime) before we call destructors. */ PyThreadState *list = _PyThreadState_RemoveExcept(tstate); _PyEval_StartTheWorldAll(runtime); - daemon_threads_exited(threading_module); + daemon_threads_exited(tstate); _PyThreadState_DeleteList(list); /* At this point no Python code should be running at all. @@ -2390,7 +2390,7 @@ Py_EndInterpreter(PyThreadState *tstate) interp->finalizing = 1; // Wrap up existing "threading"-module-created, non-daemon threads. - PyObject *threading_module = wait_for_thread_shutdown(tstate); + wait_for_thread_shutdown(tstate); // Make any remaining pending calls. _Py_FinishPendingCalls(tstate); @@ -2405,7 +2405,7 @@ Py_EndInterpreter(PyThreadState *tstate) when they attempt to take the GIL (ex: PyEval_RestoreThread()). */ _PyInterpreterState_SetFinalizing(interp, tstate); - daemon_threads_exited(threading_module); + daemon_threads_exited(tstate); // XXX Call something like _PyImport_Disable() here? _PyImport_FiniExternal(tstate->interp); @@ -3330,13 +3330,8 @@ Py_ExitStatusException(PyStatus status) /* Wait until threading._shutdown completes, provided the threading module was imported in the first place. The shutdown routine will wait until all non-daemon - "threading" threads have completed. - - Returns a reference to the threading module. `daemon_threads_exited` should - be called with this reference after the interpreter has been marked - finalizing. -*/ -static PyObject * + "threading" threads have completed. */ +static void wait_for_thread_shutdown(PyThreadState *tstate) { PyObject *result; @@ -3346,7 +3341,7 @@ wait_for_thread_shutdown(PyThreadState *tstate) PyErr_FormatUnraisable("Exception ignored on threading shutdown"); } /* else: threading not imported */ - return NULL; + return; } result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown)); if (result == NULL) { @@ -3355,24 +3350,32 @@ wait_for_thread_shutdown(PyThreadState *tstate) else { Py_DECREF(result); } - return threading; + Py_DECREF(threading); } -/* Steals a reference to threading_module */ +/* gh-123940: Mark remaining daemon threads as exited so that they may + * be joined from finalizers. + */ static void -daemon_threads_exited(PyObject *threading_module) +daemon_threads_exited(PyThreadState *tstate) { - if (threading_module == NULL) { + PyObject *threading = PyImport_GetModule(&_Py_ID(threading)); + if (threading == NULL) { + if (_PyErr_Occurred(tstate)) { + PyErr_FormatUnraisable( + "Exception ignored while marking daemon threads exited"); + } + /* else: threading not imported */ return; } - PyObject *result = PyObject_CallMethodNoArgs( - threading_module, &_Py_ID(_daemon_threads_exited)); + PyObject *result = + PyObject_CallMethodNoArgs(threading, &_Py_ID(_daemon_threads_exited)); if (result == NULL) { PyErr_FormatUnraisable( "Exception ignored while marking daemon threads exited"); } Py_XDECREF(result); - Py_DECREF(threading_module); + Py_DECREF(threading); } int Py_AtExit(void (*func)(void)) From 9a94905d266dbb4be1d82e51501513910a204d13 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 18 Sep 2024 12:47:18 -0700 Subject: [PATCH 11/13] Don't call into Python to mark daemon threads completed --- Include/internal/pycore_interp.h | 6 +++ Include/internal/pycore_pythread.h | 2 + Lib/threading.py | 1 - Modules/_threadmodule.c | 74 +++++++++++------------------- Python/pylifecycle.c | 29 +----------- Python/pystate.c | 4 +- 6 files changed, 38 insertions(+), 78 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index a1c1dd0c957230..6ef7079ac6fc4a 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -139,6 +139,12 @@ struct _is { or the size specified by the THREAD_STACK_SIZE macro. */ /* Used in Python/thread.c. */ size_t stacksize; + + /* Linked lists of ThreadHandles for threads created by + the threading module. + */ + struct llist_node non_daemon_handles; + struct llist_node daemon_handles; } threads; /* Reference to the _PyRuntime global variable. This field exists diff --git a/Include/internal/pycore_pythread.h b/Include/internal/pycore_pythread.h index f3f5942444e851..16fa05da85b3cd 100644 --- a/Include/internal/pycore_pythread.h +++ b/Include/internal/pycore_pythread.h @@ -95,6 +95,8 @@ extern int _PyThread_at_fork_reinit(PyThread_type_lock *lock); extern void _PyThread_AfterFork(struct _pythread_runtime_state *state); #endif /* HAVE_FORK */ +extern void _PyThread_DaemonThreadsForceKilled(PyInterpreterState *interp); +extern void _PyThread_ClearThreadHandles(PyInterpreterState *interp); // unset: -1 seconds, in nanoseconds #define PyThread_UNSET_TIMEOUT ((PyTime_t)(-1 * 1000 * 1000 * 1000)) diff --git a/Lib/threading.py b/Lib/threading.py index 14ea29c0cfab1c..94ea2f08178369 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -42,7 +42,6 @@ get_ident = _thread.get_ident _get_main_thread_ident = _thread._get_main_thread_ident _is_main_interpreter = _thread._is_main_interpreter -_daemon_threads_exited = _thread._daemon_threads_exited try: get_native_id = _thread.get_native_id _HAVE_THREAD_NATIVE_ID = True diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 5903fbc1466350..25c59a00479df0 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -30,14 +30,6 @@ typedef struct { PyTypeObject *local_type; PyTypeObject *local_dummy_type; PyTypeObject *thread_handle_type; - - // Linked list of handles to all non-daemon threads created by the - // threading module. We wait for these to finish at shutdown. - struct llist_node non_daemon_handles; - - // Linked list of handles to daemon threads created by the threading - // module. We set the events in these handles as done at shutdown. - struct llist_node daemon_handles; } thread_module_state; static inline thread_module_state* @@ -82,8 +74,8 @@ typedef enum { typedef struct { struct llist_node node; // linked list node (see _pythread_runtime_state) - // belongs to either `non_daemon_handles` or `daemon_handles` (see - // thread_module_state) + // belongs to either `non_daemon_handles` or `daemon_handles` on + // PyInterpreterState struct llist_node shutdown_node; // The `ident`, `os_handle`, `has_os_handle`, and `state` fields are @@ -156,15 +148,18 @@ add_to_shutdown_handles(struct llist_node *shutdown_handles, HEAD_UNLOCK(&_PyRuntime); } -static void -clear_shutdown_handles(thread_module_state *state) +// Remove any remaining handles (e.g. if shutdown exited early due to +// interrupt) so that attempts to unlink the handle after our module state +// is destroyed do not crash. +void +_PyThread_ClearThreadHandles(PyInterpreterState *interp) { HEAD_LOCK(&_PyRuntime); struct llist_node *node; - llist_for_each_safe(node, &state->daemon_handles) { + llist_for_each_safe(node, &interp->threads.daemon_handles) { llist_remove(node); } - llist_for_each_safe(node, &state->non_daemon_handles) { + llist_for_each_safe(node, &interp->threads.non_daemon_handles) { llist_remove(node); } HEAD_UNLOCK(&_PyRuntime); @@ -1872,8 +1867,8 @@ Return True if daemon threads are allowed in the current interpreter,\n\ and False otherwise.\n"); static int -do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args, - PyObject *kwargs, ThreadHandle *handle, int daemon) +do_start_new_thread(PyObject *func, PyObject *args, PyObject *kwargs, + ThreadHandle *handle, int daemon) { PyInterpreterState *interp = _PyInterpreterState_GET(); if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_THREADS)) { @@ -1887,9 +1882,9 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args, return -1; } - struct llist_node *shutdown_handles = &state->non_daemon_handles; + struct llist_node *shutdown_handles = &interp->threads.non_daemon_handles; if (daemon) { - shutdown_handles = &state->daemon_handles; + shutdown_handles = &interp->threads.daemon_handles; } if (ThreadHandle_start(handle, func, args, kwargs, shutdown_handles) < 0) { @@ -1903,7 +1898,6 @@ static PyObject * thread_PyThread_start_new_thread(PyObject *module, PyObject *fargs) { PyObject *func, *args, *kwargs = NULL; - thread_module_state *state = get_thread_state(module); if (!PyArg_UnpackTuple(fargs, "start_new_thread", 2, 3, &func, &args, &kwargs)) @@ -1934,8 +1928,7 @@ thread_PyThread_start_new_thread(PyObject *module, PyObject *fargs) return NULL; } - int st = - do_start_new_thread(state, func, args, kwargs, handle, /*daemon=*/1); + int st = do_start_new_thread(func, args, kwargs, handle, /*daemon=*/1); if (st < 0) { ThreadHandle_decref(handle); return NULL; @@ -2012,8 +2005,9 @@ thread_PyThread_start_joinable_thread(PyObject *module, PyObject *fargs, if (args == NULL) { return NULL; } - int st = do_start_new_thread(state, func, args, - /*kwargs=*/ NULL, ((PyThreadHandleObject*)hobj)->handle, daemon); + int st = do_start_new_thread( + func, args, + /*kwargs=*/NULL, ((PyThreadHandleObject *)hobj)->handle, daemon); Py_DECREF(args); if (st < 0) { Py_DECREF(hobj); @@ -2374,7 +2368,7 @@ static PyObject * thread_shutdown(PyObject *self, PyObject *args) { PyThread_ident_t ident = PyThread_get_thread_ident_ex(); - thread_module_state *state = get_thread_state(self); + PyInterpreterState *interp = _PyInterpreterState_GET(); for (;;) { ThreadHandle *handle = NULL; @@ -2382,7 +2376,7 @@ thread_shutdown(PyObject *self, PyObject *args) // Find a thread that's not yet finished. HEAD_LOCK(&_PyRuntime); struct llist_node *node; - llist_for_each_safe(node, &state->non_daemon_handles) { + llist_for_each_safe(node, &interp->threads.non_daemon_handles) { ThreadHandle *cur = llist_data(node, ThreadHandle, shutdown_node); if (cur->ident != ident) { ThreadHandle_incref(cur); @@ -2417,18 +2411,19 @@ PyDoc_STRVAR(shutdown_doc, \n\ Wait for all non-daemon threads (other than the calling thread) to stop."); -static PyObject * -thread__daemon_threads_exited(PyObject *self, PyObject *args) +/* gh-123940: Mark remaining daemon threads as exited so that they may + * be joined from finalizers. + */ +void +_PyThread_DaemonThreadsForceKilled(PyInterpreterState *interp) { - thread_module_state *state = get_thread_state(self); - for (;;) { HEAD_LOCK(&_PyRuntime); - if (llist_empty(&state->daemon_handles)) { + if (llist_empty(&interp->threads.daemon_handles)) { HEAD_UNLOCK(&_PyRuntime); break; } - struct llist_node *node = state->daemon_handles.next; + struct llist_node *node = interp->threads.daemon_handles.next; ThreadHandle *handle = llist_data(node, ThreadHandle, shutdown_node); ThreadHandle_incref(handle); llist_remove(node); @@ -2438,16 +2433,8 @@ thread__daemon_threads_exited(PyObject *self, PyObject *args) _PyEvent_Notify(&handle->thread_is_exiting); ThreadHandle_decref(handle); } - - Py_RETURN_NONE; } -PyDoc_STRVAR(_daemon_threads_exited_doc, -"_daemon_threads_exited($module, /)\n\ ---\n\ -\n\ -Mark that daemon threads have exited."); - static PyObject * thread__make_thread_handle(PyObject *module, PyObject *identobj) { @@ -2527,8 +2514,6 @@ static PyMethodDef thread_methods[] = { METH_NOARGS, thread__is_main_interpreter_doc}, {"_shutdown", thread_shutdown, METH_NOARGS, shutdown_doc}, - {"_daemon_threads_exited", thread__daemon_threads_exited, - METH_NOARGS, _daemon_threads_exited_doc}, {"_make_thread_handle", thread__make_thread_handle, METH_O, thread__make_thread_handle_doc}, {"_get_main_thread_ident", thread__get_main_thread_ident, @@ -2622,9 +2607,6 @@ thread_module_exec(PyObject *module) return -1; } - llist_init(&state->daemon_handles); - llist_init(&state->non_daemon_handles); - return 0; } @@ -2650,10 +2632,6 @@ thread_module_clear(PyObject *module) Py_CLEAR(state->local_type); Py_CLEAR(state->local_dummy_type); Py_CLEAR(state->thread_handle_type); - // Remove any remaining handles (e.g. if shutdown exited early due to - // interrupt) so that attempts to unlink the handle after our module state - // is destroyed do not crash. - clear_shutdown_handles(state); return 0; } diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 0bd6d8ba678f49..89bf1aa117ef81 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -75,7 +75,6 @@ static PyStatus init_sys_streams(PyThreadState *tstate); static PyStatus init_android_streams(PyThreadState *tstate); #endif static void wait_for_thread_shutdown(PyThreadState *tstate); -static void daemon_threads_exited(PyThreadState *tstate); static void finalize_subinterpreters(void); static void call_ll_exitfuncs(_PyRuntimeState *runtime); @@ -2041,7 +2040,7 @@ _Py_Finalize(_PyRuntimeState *runtime) before we call destructors. */ PyThreadState *list = _PyThreadState_RemoveExcept(tstate); _PyEval_StartTheWorldAll(runtime); - daemon_threads_exited(tstate); + _PyThread_DaemonThreadsForceKilled(tstate->interp); _PyThreadState_DeleteList(list); /* At this point no Python code should be running at all. @@ -2405,7 +2404,6 @@ Py_EndInterpreter(PyThreadState *tstate) when they attempt to take the GIL (ex: PyEval_RestoreThread()). */ _PyInterpreterState_SetFinalizing(interp, tstate); - daemon_threads_exited(tstate); // XXX Call something like _PyImport_Disable() here? _PyImport_FiniExternal(tstate->interp); @@ -3353,31 +3351,6 @@ wait_for_thread_shutdown(PyThreadState *tstate) Py_DECREF(threading); } -/* gh-123940: Mark remaining daemon threads as exited so that they may - * be joined from finalizers. - */ -static void -daemon_threads_exited(PyThreadState *tstate) -{ - PyObject *threading = PyImport_GetModule(&_Py_ID(threading)); - if (threading == NULL) { - if (_PyErr_Occurred(tstate)) { - PyErr_FormatUnraisable( - "Exception ignored while marking daemon threads exited"); - } - /* else: threading not imported */ - return; - } - PyObject *result = - PyObject_CallMethodNoArgs(threading, &_Py_ID(_daemon_threads_exited)); - if (result == NULL) { - PyErr_FormatUnraisable( - "Exception ignored while marking daemon threads exited"); - } - Py_XDECREF(result); - Py_DECREF(threading); -} - int Py_AtExit(void (*func)(void)) { struct _atexit_runtime_state *state = &_PyRuntime.atexit; diff --git a/Python/pystate.c b/Python/pystate.c index 54caf373e91d6c..24a0b2558c677e 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -665,6 +665,8 @@ init_interpreter(PyInterpreterState *interp, /* Fix the self-referential, statically initialized fields. */ interp->dtoa = (struct _dtoa_state)_dtoa_state_INIT(interp); } + llist_init(&interp->threads.daemon_handles); + llist_init(&interp->threads.non_daemon_handles); interp->_initialized = 1; return _PyStatus_OK(); @@ -811,7 +813,7 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) // XXX Eliminate the need to do this. tstate->_status.cleared = 0; } - + _PyThread_ClearThreadHandles(interp); #ifdef _Py_TIER2 _PyOptimizerObject *old = _Py_SetOptimizer(interp, NULL); assert(old != NULL); From 512b5fc98b5a5c8a5b8691c2b8fd105fc4814881 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 18 Sep 2024 13:57:00 -0700 Subject: [PATCH 12/13] Regen global objects --- Include/internal/pycore_global_objects_fini_generated.h | 1 - Include/internal/pycore_global_strings.h | 1 - Include/internal/pycore_runtime_init_generated.h | 1 - Include/internal/pycore_unicodeobject_generated.h | 4 ---- 4 files changed, 7 deletions(-) diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index 0c8319520c5d21..6e948e16b7dbe8 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -745,7 +745,6 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_blksize)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_bootstrap)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_check_retval_)); - _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_daemon_threads_exited)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_dealloc_warn)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_feature_version)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_field_types)); diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index acd3a9b7043427..5c63a6e519b93d 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -234,7 +234,6 @@ struct _Py_global_strings { STRUCT_FOR_ID(_blksize) STRUCT_FOR_ID(_bootstrap) STRUCT_FOR_ID(_check_retval_) - STRUCT_FOR_ID(_daemon_threads_exited) STRUCT_FOR_ID(_dealloc_warn) STRUCT_FOR_ID(_feature_version) STRUCT_FOR_ID(_field_types) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index 57d6eef73dbcf5..bac6b5b8fcfd9d 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -743,7 +743,6 @@ extern "C" { INIT_ID(_blksize), \ INIT_ID(_bootstrap), \ INIT_ID(_check_retval_), \ - INIT_ID(_daemon_threads_exited), \ INIT_ID(_dealloc_warn), \ INIT_ID(_feature_version), \ INIT_ID(_field_types), \ diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h index c4ef3bbbbee3cf..efdbde4c8ea3c6 100644 --- a/Include/internal/pycore_unicodeobject_generated.h +++ b/Include/internal/pycore_unicodeobject_generated.h @@ -736,10 +736,6 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); assert(PyUnicode_GET_LENGTH(string) != 1); - string = &_Py_ID(_daemon_threads_exited); - _PyUnicode_InternStatic(interp, &string); - assert(_PyUnicode_CheckConsistency(string, 1)); - assert(PyUnicode_GET_LENGTH(string) != 1); string = &_Py_ID(_dealloc_warn); _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); From 70f28c8af75a85c39fdc0d8914f91f10c3bdfe5c Mon Sep 17 00:00:00 2001 From: Matt Page Date: Wed, 18 Sep 2024 14:03:47 -0700 Subject: [PATCH 13/13] Maybe fix windows build Include threadmodule when building _freeze_module --- PCbuild/_freeze_module.vcxproj | 1 + PCbuild/_freeze_module.vcxproj.filters | 3 +++ 2 files changed, 4 insertions(+) diff --git a/PCbuild/_freeze_module.vcxproj b/PCbuild/_freeze_module.vcxproj index 743e6e2a66a8f1..c2894c5eb63c3e 100644 --- a/PCbuild/_freeze_module.vcxproj +++ b/PCbuild/_freeze_module.vcxproj @@ -111,6 +111,7 @@ + diff --git a/PCbuild/_freeze_module.vcxproj.filters b/PCbuild/_freeze_module.vcxproj.filters index 0887a47917a931..26975767a339f7 100644 --- a/PCbuild/_freeze_module.vcxproj.filters +++ b/PCbuild/_freeze_module.vcxproj.filters @@ -431,6 +431,9 @@ Source Files + + Source Files + Source Files