From 37baa8fb9af1d2a68c5350d5d8f781ad095ed864 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 15 Oct 2026 14:40:22 -0600 Subject: [PATCH 1/3] gh-159485: Fix warnings lock deadlock during interpreter shutdown --- Include/internal/pycore_lock.h | 3 + Include/internal/pycore_warnings.h | 2 + Lib/test/test_embed.py | 62 +++++++++++++++++++ ...-10-15-14-41-28.gh-issue-159485.S3IOyb.rst | 2 + Modules/_testinternalcapi/test_lock.c | 16 +++++ Python/_warnings.c | 7 +++ Python/lock.c | 15 +++++ Python/pylifecycle.c | 2 + 8 files changed, 109 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-14-41-28.gh-issue-159485.S3IOyb.rst diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index 7dd2419ececf299..f8869afb2e7d299 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -168,6 +168,9 @@ typedef struct { size_t level; } _PyRecursiveMutex; +// All other threads that use the mutex must be barred from reattaching. +extern void _PyRecursiveMutex_at_shutdown_reinit(_PyRecursiveMutex *m); + PyAPI_FUNC(int) _PyRecursiveMutex_IsLockedByCurrentThread(_PyRecursiveMutex *m); PyAPI_FUNC(void) _PyRecursiveMutex_Lock(_PyRecursiveMutex *m); extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags); diff --git a/Include/internal/pycore_warnings.h b/Include/internal/pycore_warnings.h index e1348d529a22463..45f836300a6cd63 100644 --- a/Include/internal/pycore_warnings.h +++ b/Include/internal/pycore_warnings.h @@ -8,6 +8,8 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif +extern void _PyWarnings_PrepareForFinalization(PyInterpreterState *interp); + extern int _PyWarnings_InitState(PyInterpreterState *interp); extern PyObject* _PyWarnings_Init(void); diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index 02fb352502b54dc..eabe95cc99b4b79 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -507,6 +507,68 @@ def test_datetime_reset_strptime(self): out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) self.assertEqual(out, '20000101\n' * INIT_LOOPS) + @threading_helper.requires_working_threading() + @unittest.skipUnless(_testinternalcapi, "requires _testinternalcapi") + def test_warnings_at_shutdown(self): + import_helper.import_module('_testcapi') + code = textwrap.dedent(''' + import _testinternalcapi + import _warnings + import threading + import warnings + from test import support + + warnings.simplefilter('ignore') + local = threading.local() + ready = threading.Event() + + class WarnOnDelete: + def __del__(self): + warnings.warn('during shutdown', ResourceWarning) + if not ready.is_set(): + # The finalizing thread still owns all three levels. + for _ in range(3): + _warnings._release_lock() + try: + _warnings._release_lock() + except RuntimeError: + pass + else: + raise AssertionError('unexpected recursion depth') + print('finalized') + + def worker(): + local.obj = WarnOnDelete() + _warnings._acquire_lock() + ready.set() + threading.Event().wait() + + for _ in range(3): + _warnings._acquire_lock() + threading.Thread(target=worker, daemon=True).start() + for _ in support.sleeping_retry(4): + if _testinternalcapi.warnings_lock_has_waiters(): + break + ''') + release = textwrap.dedent(''' + for _ in range(3): + _warnings._release_lock() + assert ready.wait(4), 'lock not acquired' + ''') + subinterp = ( + 'from test import support\n' + f'assert support.run_in_subinterp({code + release!r}) == 0' + ) + # Cover Py_EndInterpreter(), then shut down with a waiter. Reuse the + # main interpreter's mutex and shut down with a daemon owning it. + # All three thread-local finalizers must warn. + out, err = self.run_embedded_interpreter( + "test_repeated_init_exec", subinterp + '\n' + code, code + release, + env=remove_python_envvars(), + timeout=10) + self.assertEqual(out, 'finalized\n' * 3) + self.assertEqual(err, '--- Loop #1 ---\n--- Loop #2 ---\n') + def test_static_types_inherited_slots(self): script = textwrap.dedent(""" import test.support diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-14-41-28.gh-issue-159485.S3IOyb.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-14-41-28.gh-issue-159485.S3IOyb.rst new file mode 100644 index 000000000000000..55966c3718aade3 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-14-41-28.gh-issue-159485.S3IOyb.rst @@ -0,0 +1,2 @@ +Prevent a deadlock when emitting warnings during late interpreter shutdown +if a daemon thread holds the warnings lock or is waiting to acquire it. diff --git a/Modules/_testinternalcapi/test_lock.c b/Modules/_testinternalcapi/test_lock.c index 596120ef2751963..df684922d5c7545 100644 --- a/Modules/_testinternalcapi/test_lock.c +++ b/Modules/_testinternalcapi/test_lock.c @@ -1,7 +1,9 @@ // C Extension module to test pycore_lock.h API #include "parts.h" +#include "pycore_interp.h" // PyInterpreterState.warnings #include "pycore_lock.h" +#include "pycore_pystate.h" // _PyEval_StopTheWorld() #include "pycore_pythread.h" // PyThread_get_thread_ident_ex() #include "clinic/test_lock.c.h" @@ -549,7 +551,21 @@ test_lock_recursive(PyObject *self, PyObject *obj) Py_RETURN_NONE; } +static PyObject * +warnings_lock_has_waiters(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyInterpreterState *interp = PyInterpreterState_Get(); + PyMutex *mutex = &interp->warnings.lock.mutex; + // A contender sets the parked bit before enqueuing, but cannot detach + // until it has enqueued. Stop the world to close that window on nogil. + _PyEval_StopTheWorld(interp); + uint8_t bits = _Py_atomic_load_uint8_relaxed(&mutex->_bits); + _PyEval_StartTheWorld(interp); + return PyBool_FromLong((bits & _Py_HAS_PARKED) != 0); +} + static PyMethodDef test_methods[] = { + {"warnings_lock_has_waiters", warnings_lock_has_waiters, METH_NOARGS}, {"test_lock_basic", test_lock_basic, METH_NOARGS}, {"test_lock_two_threads", test_lock_two_threads, METH_NOARGS}, {"test_lock_counter", test_lock_counter, METH_NOARGS}, diff --git a/Python/_warnings.c b/Python/_warnings.c index 586cec195fac680..773eb33ef2cf26d 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -254,6 +254,13 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import) return obj; } +void +_PyWarnings_PrepareForFinalization(PyInterpreterState *interp) +{ + WarningsState *st = warnings_get_state(interp); + _PyRecursiveMutex_at_shutdown_reinit(&st->lock); +} + static inline void warnings_lock(PyInterpreterState *interp) { diff --git a/Python/lock.c b/Python/lock.c index b669e3dcdbc9993..967ab80df2362c6 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -427,6 +427,21 @@ _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags return s; } +void +_PyRecursiveMutex_at_shutdown_reinit(_PyRecursiveMutex *m) +{ + // Preserve the finalizing thread's ownership and recursion depth. + if (_PyRecursiveMutex_IsLockedByCurrentThread(m)) { + m->mutex = (PyMutex){._bits = _Py_LOCKED}; + } + else { + memset(m, 0, sizeof(*m)); + } + // Remove parked waiters from the queue. After waking, they will hang + // when they try to reattach their thread state. + _PyParkingLot_UnparkAll(&m->mutex._bits); +} + void _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m) { diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index ba1b94c47984ab6..032cb1f9d00b759 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2452,6 +2452,7 @@ _Py_Finalize(_PyRuntimeState *runtime) for (PyThreadState *p = list; p != NULL; p = p->next) { _PyThreadState_SetShuttingDown(p); } + _PyWarnings_PrepareForFinalization(tstate->interp); _PyEval_StartTheWorldAll(runtime); PyMutex_Unlock(&tstate->interp->ceval.pending.mutex); @@ -2834,6 +2835,7 @@ Py_EndInterpreter(PyThreadState *tstate) _PyThreadState_SetShuttingDown(p); } + _PyWarnings_PrepareForFinalization(interp); _PyEval_StartTheWorldAll(interp->runtime); PyMutex_Unlock(&interp->ceval.pending.mutex); _PyThreadState_DeleteList(list, /*is_after_fork=*/0); From fdec193a5878d772dd7f4640c8aa4d39d516022b Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 16 Oct 2026 09:10:21 -0600 Subject: [PATCH 2/3] gh-159485: Hold an interpreter guard while using the warnings lock --- Include/internal/pycore_interp_structs.h | 2 + Include/internal/pycore_lock.h | 3 - Include/internal/pycore_warnings.h | 2 - Lib/test/test_embed.py | 69 +++++-------- ...-10-15-14-41-28.gh-issue-159485.S3IOyb.rst | 4 +- Modules/_testinternalcapi/test_lock.c | 16 --- Python/_warnings.c | 98 ++++++++++++++----- Python/lock.c | 15 --- Python/pylifecycle.c | 2 - 9 files changed, 103 insertions(+), 108 deletions(-) diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index 9cc25eda44f9de1..0d1f08ea3569144 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -652,6 +652,8 @@ struct _warnings_runtime_state { _PyRecursiveMutex lock; long filters_version; PyObject *context; + // The lock owner's guard must outlive the mutex unlock. + PyInterpreterGuard *lock_guard; }; struct _Py_mem_interp_free_queue { diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index f8869afb2e7d299..7dd2419ececf299 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -168,9 +168,6 @@ typedef struct { size_t level; } _PyRecursiveMutex; -// All other threads that use the mutex must be barred from reattaching. -extern void _PyRecursiveMutex_at_shutdown_reinit(_PyRecursiveMutex *m); - PyAPI_FUNC(int) _PyRecursiveMutex_IsLockedByCurrentThread(_PyRecursiveMutex *m); PyAPI_FUNC(void) _PyRecursiveMutex_Lock(_PyRecursiveMutex *m); extern PyLockStatus _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags flags); diff --git a/Include/internal/pycore_warnings.h b/Include/internal/pycore_warnings.h index 45f836300a6cd63..e1348d529a22463 100644 --- a/Include/internal/pycore_warnings.h +++ b/Include/internal/pycore_warnings.h @@ -8,8 +8,6 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif -extern void _PyWarnings_PrepareForFinalization(PyInterpreterState *interp); - extern int _PyWarnings_InitState(PyInterpreterState *interp); extern PyObject* _PyWarnings_Init(void); diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index eabe95cc99b4b79..0c1f05b1f73beae 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -508,65 +508,46 @@ def test_datetime_reset_strptime(self): self.assertEqual(out, '20000101\n' * INIT_LOOPS) @threading_helper.requires_working_threading() - @unittest.skipUnless(_testinternalcapi, "requires _testinternalcapi") def test_warnings_at_shutdown(self): - import_helper.import_module('_testcapi') code = textwrap.dedent(''' - import _testinternalcapi - import _warnings import threading + import time import warnings - from test import support - warnings.simplefilter('ignore') - local = threading.local() + warnings.simplefilter('always', ResourceWarning) ready = threading.Event() class WarnOnDelete: + def __init__(self): + self.self_ref = self + def __del__(self): - warnings.warn('during shutdown', ResourceWarning) - if not ready.is_set(): - # The finalizing thread still owns all three levels. - for _ in range(3): - _warnings._release_lock() - try: - _warnings._release_lock() - except RuntimeError: - pass - else: - raise AssertionError('unexpected recursion depth') - print('finalized') + warnings.warn('late', ResourceWarning) + + def showwarning(message, *args, **kwargs): + if str(message) == 'hold': + warnings.warn('nested', ResourceWarning) + ready.set() + # A Python event cannot synchronize with the point where + # shutdown stops daemon threads, so hold the lock briefly. + time.sleep(0.1) + else: + print(message) + + warnings.showwarning = showwarning def worker(): - local.obj = WarnOnDelete() - _warnings._acquire_lock() - ready.set() - threading.Event().wait() + warnings.warn('hold', ResourceWarning) - for _ in range(3): - _warnings._acquire_lock() threading.Thread(target=worker, daemon=True).start() - for _ in support.sleeping_retry(4): - if _testinternalcapi.warnings_lock_has_waiters(): - break - ''') - release = textwrap.dedent(''' - for _ in range(3): - _warnings._release_lock() - assert ready.wait(4), 'lock not acquired' + assert ready.wait(4), 'daemon did not start warning' + # Collect the cycle during shutdown GC, after daemon threads stop. + WarnOnDelete() ''') - subinterp = ( - 'from test import support\n' - f'assert support.run_in_subinterp({code + release!r}) == 0' - ) - # Cover Py_EndInterpreter(), then shut down with a waiter. Reuse the - # main interpreter's mutex and shut down with a daemon owning it. - # All three thread-local finalizers must warn. out, err = self.run_embedded_interpreter( - "test_repeated_init_exec", subinterp + '\n' + code, code + release, - env=remove_python_envvars(), - timeout=10) - self.assertEqual(out, 'finalized\n' * 3) + "test_repeated_init_exec", code, "pass", + env=remove_python_envvars(), timeout=10) + self.assertEqual(out, 'nested\nlate\n') self.assertEqual(err, '--- Loop #1 ---\n--- Loop #2 ---\n') def test_static_types_inherited_slots(self): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-14-41-28.gh-issue-159485.S3IOyb.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-14-41-28.gh-issue-159485.S3IOyb.rst index 55966c3718aade3..f49806a97164bc8 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-14-41-28.gh-issue-159485.S3IOyb.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-10-15-14-41-28.gh-issue-159485.S3IOyb.rst @@ -1,2 +1,2 @@ -Prevent a deadlock when emitting warnings during late interpreter shutdown -if a daemon thread holds the warnings lock or is waiting to acquire it. +Prevent a shutdown deadlock when a daemon thread emits a warning. The +interpreter now waits for the thread to release the warnings lock. diff --git a/Modules/_testinternalcapi/test_lock.c b/Modules/_testinternalcapi/test_lock.c index df684922d5c7545..596120ef2751963 100644 --- a/Modules/_testinternalcapi/test_lock.c +++ b/Modules/_testinternalcapi/test_lock.c @@ -1,9 +1,7 @@ // C Extension module to test pycore_lock.h API #include "parts.h" -#include "pycore_interp.h" // PyInterpreterState.warnings #include "pycore_lock.h" -#include "pycore_pystate.h" // _PyEval_StopTheWorld() #include "pycore_pythread.h" // PyThread_get_thread_ident_ex() #include "clinic/test_lock.c.h" @@ -551,21 +549,7 @@ test_lock_recursive(PyObject *self, PyObject *obj) Py_RETURN_NONE; } -static PyObject * -warnings_lock_has_waiters(PyObject *self, PyObject *Py_UNUSED(ignored)) -{ - PyInterpreterState *interp = PyInterpreterState_Get(); - PyMutex *mutex = &interp->warnings.lock.mutex; - // A contender sets the parked bit before enqueuing, but cannot detach - // until it has enqueued. Stop the world to close that window on nogil. - _PyEval_StopTheWorld(interp); - uint8_t bits = _Py_atomic_load_uint8_relaxed(&mutex->_bits); - _PyEval_StartTheWorld(interp); - return PyBool_FromLong((bits & _Py_HAS_PARKED) != 0); -} - static PyMethodDef test_methods[] = { - {"warnings_lock_has_waiters", warnings_lock_has_waiters, METH_NOARGS}, {"test_lock_basic", test_lock_basic, METH_NOARGS}, {"test_lock_two_threads", test_lock_two_threads, METH_NOARGS}, {"test_lock_counter", test_lock_counter, METH_NOARGS}, diff --git a/Python/_warnings.c b/Python/_warnings.c index 773eb33ef2cf26d..701c51320de9278 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -2,6 +2,7 @@ #include "pycore_frame.h" // PyFrameObject #include "pycore_genobject.h" // PyAsyncGenObject #include "pycore_import.h" // _PyImport_GetModules() +#include "pycore_interp.h" // _PyInterpreterState_GetFinalizing() #include "pycore_interpframe.h" // _PyFrame_GetCode() #include "pycore_long.h" // _PyLong_GetZero() #include "pycore_pylifecycle.h" // _Py_IsInterpreterFinalizing() @@ -254,19 +255,39 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import) return obj; } -void -_PyWarnings_PrepareForFinalization(PyInterpreterState *interp) -{ - WarningsState *st = warnings_get_state(interp); - _PyRecursiveMutex_at_shutdown_reinit(&st->lock); -} +static _Py_thread_local PyInterpreterGuard warnings_guard; -static inline void +static inline int warnings_lock(PyInterpreterState *interp) { WarningsState *st = warnings_get_state(interp); assert(st != NULL); + if (_PyRecursiveMutex_IsLockedByCurrentThread(&st->lock)) { + _PyRecursiveMutex_Lock(&st->lock); + return 0; + } + PyInterpreterGuard *guard = NULL; + if (_PyInterpreterState_GetFinalizing(interp) != _PyThreadState_GET()) { + guard = &warnings_guard; + if (guard->interp != NULL) { + guard = PyMem_RawMalloc(sizeof(*guard)); + if (guard == NULL) { + PyErr_NoMemory(); + return -1; + } + } + if (_PyInterpreterGuard_TryAcquire(interp, guard) < 0) { + if (guard != &warnings_guard) { + PyMem_RawFree(guard); + } + // Finalization has begun; this thread must stop before it takes + // the lock. + _PyThreadState_HangThread(_PyThreadState_GET()); + } + } _PyRecursiveMutex_Lock(&st->lock); + st->lock_guard = guard; + return 0; } static inline int @@ -274,7 +295,26 @@ warnings_unlock(PyInterpreterState *interp) { WarningsState *st = warnings_get_state(interp); assert(st != NULL); - return _PyRecursiveMutex_TryUnlock(&st->lock); + if (!_PyRecursiveMutex_IsLockedByCurrentThread(&st->lock)) { + return -1; + } + if (st->lock.level > 0) { + _PyRecursiveMutex_Unlock(&st->lock); + return 0; + } + PyInterpreterGuard *guard = st->lock_guard; + st->lock_guard = NULL; + _PyRecursiveMutex_Unlock(&st->lock); + if (guard != NULL) { + _PyInterpreterGuard_Release(guard); + if (guard == &warnings_guard) { + guard->interp = NULL; + } + else { + PyMem_RawFree(guard); + } + } + return 0; } static inline bool @@ -358,7 +398,9 @@ warnings_acquire_lock_impl(PyObject *module) if (interp == NULL) { return NULL; } - warnings_lock(interp); + if (warnings_lock(interp) < 0) { + return NULL; + } Py_RETURN_NONE; } @@ -1140,10 +1182,14 @@ do_warn(PyObject *message, PyObject *category, Py_ssize_t stack_level, &filename, &lineno, &module, ®istry)) return NULL; - warnings_lock(tstate->interp); - res = warn_explicit(tstate, category, message, filename, lineno, module, registry, - NULL, source); - warnings_unlock(tstate->interp); + if (warnings_lock(tstate->interp) < 0) { + res = NULL; + } + else { + res = warn_explicit(tstate, category, message, filename, lineno, module, registry, + NULL, source); + warnings_unlock(tstate->interp); + } Py_DECREF(filename); Py_DECREF(registry); Py_DECREF(module); @@ -1322,10 +1368,14 @@ warnings_warn_explicit_impl(PyObject *module, PyObject *message, } } - warnings_lock(tstate->interp); - returned = warn_explicit(tstate, category, message, filename, lineno, - mod, registry, source_line, sourceobj); - warnings_unlock(tstate->interp); + if (warnings_lock(tstate->interp) < 0) { + returned = NULL; + } + else { + returned = warn_explicit(tstate, category, message, filename, lineno, + mod, registry, source_line, sourceobj); + warnings_unlock(tstate->interp); + } Py_XDECREF(source_line); return returned; } @@ -1472,7 +1522,9 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message, return -1; } - warnings_lock(tstate->interp); + if (warnings_lock(tstate->interp) < 0) { + return -1; + } res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, NULL); warnings_unlock(tstate->interp); @@ -1537,19 +1589,17 @@ PyErr_WarnExplicitFormat(PyObject *category, va_start(vargs, format); message = PyUnicode_FromFormatV(format, vargs); if (message != NULL) { - PyObject *res; PyThreadState *tstate = get_current_tstate(); - if (tstate != NULL) { - warnings_lock(tstate->interp); - res = warn_explicit(tstate, category, message, filename, lineno, - module, registry, NULL, NULL); + if (tstate != NULL && warnings_lock(tstate->interp) == 0) { + PyObject *res = warn_explicit(tstate, category, message, filename, lineno, + module, registry, NULL, NULL); warnings_unlock(tstate->interp); - Py_DECREF(message); if (res != NULL) { Py_DECREF(res); ret = 0; } } + Py_DECREF(message); } va_end(vargs); exit: diff --git a/Python/lock.c b/Python/lock.c index 967ab80df2362c6..b669e3dcdbc9993 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -427,21 +427,6 @@ _PyRecursiveMutex_LockTimed(_PyRecursiveMutex *m, PyTime_t timeout, _PyLockFlags return s; } -void -_PyRecursiveMutex_at_shutdown_reinit(_PyRecursiveMutex *m) -{ - // Preserve the finalizing thread's ownership and recursion depth. - if (_PyRecursiveMutex_IsLockedByCurrentThread(m)) { - m->mutex = (PyMutex){._bits = _Py_LOCKED}; - } - else { - memset(m, 0, sizeof(*m)); - } - // Remove parked waiters from the queue. After waking, they will hang - // when they try to reattach their thread state. - _PyParkingLot_UnparkAll(&m->mutex._bits); -} - void _PyRecursiveMutex_Unlock(_PyRecursiveMutex *m) { diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 032cb1f9d00b759..ba1b94c47984ab6 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2452,7 +2452,6 @@ _Py_Finalize(_PyRuntimeState *runtime) for (PyThreadState *p = list; p != NULL; p = p->next) { _PyThreadState_SetShuttingDown(p); } - _PyWarnings_PrepareForFinalization(tstate->interp); _PyEval_StartTheWorldAll(runtime); PyMutex_Unlock(&tstate->interp->ceval.pending.mutex); @@ -2835,7 +2834,6 @@ Py_EndInterpreter(PyThreadState *tstate) _PyThreadState_SetShuttingDown(p); } - _PyWarnings_PrepareForFinalization(interp); _PyEval_StartTheWorldAll(interp->runtime); PyMutex_Unlock(&interp->ceval.pending.mutex); _PyThreadState_DeleteList(list, /*is_after_fork=*/0); From 538a8429f64d13824a5d6d69e51ea61c9cca4110 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Fri, 16 Oct 2026 11:12:52 -0600 Subject: [PATCH 3/3] gh-159485: Use the public interpreter guard API for the warnings lock --- Include/internal/pycore_interp_structs.h | 2 - Include/internal/pycore_tstate.h | 3 ++ Python/_warnings.c | 48 ++++++++---------------- 3 files changed, 18 insertions(+), 35 deletions(-) diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index 0d1f08ea3569144..9cc25eda44f9de1 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -652,8 +652,6 @@ struct _warnings_runtime_state { _PyRecursiveMutex lock; long filters_version; PyObject *context; - // The lock owner's guard must outlive the mutex unlock. - PyInterpreterGuard *lock_guard; }; struct _Py_mem_interp_free_queue { diff --git a/Include/internal/pycore_tstate.h b/Include/internal/pycore_tstate.h index 58dd66b69efc9da..2af09ad388256be 100644 --- a/Include/internal/pycore_tstate.h +++ b/Include/internal/pycore_tstate.h @@ -54,6 +54,9 @@ typedef struct _PyThreadStateImpl { // Reused for the thread's lifetime; owns placeholders while they resolve. PyObject *lazy_imports; + // Held while this thread owns the warnings lock. + PyInterpreterGuard *warnings_lock_guard; + // Distinguishes between yield and return from PyEval_EvalFrame(). // See gen_send_ex2() in Objects/genobject.c enum { diff --git a/Python/_warnings.c b/Python/_warnings.c index 701c51320de9278..8a7d5956be1a193 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -7,6 +7,7 @@ #include "pycore_long.h" // _PyLong_GetZero() #include "pycore_pylifecycle.h" // _Py_IsInterpreterFinalizing() #include "pycore_pystate.h" // _PyThreadState_GET() +#include "pycore_tstate.h" // _PyThreadStateImpl #include "pycore_traceback.h" // _Py_DisplaySourceLine() #include "pycore_tuple.h" // _PyTuple_FromPair #include "pycore_unicodeobject.h" // _PyUnicode_EqualToASCIIString() @@ -255,38 +256,24 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import) return obj; } -static _Py_thread_local PyInterpreterGuard warnings_guard; - static inline int warnings_lock(PyInterpreterState *interp) { WarningsState *st = warnings_get_state(interp); assert(st != NULL); - if (_PyRecursiveMutex_IsLockedByCurrentThread(&st->lock)) { - _PyRecursiveMutex_Lock(&st->lock); - return 0; - } - PyInterpreterGuard *guard = NULL; - if (_PyInterpreterState_GetFinalizing(interp) != _PyThreadState_GET()) { - guard = &warnings_guard; - if (guard->interp != NULL) { - guard = PyMem_RawMalloc(sizeof(*guard)); - if (guard == NULL) { - PyErr_NoMemory(); - return -1; - } - } - if (_PyInterpreterGuard_TryAcquire(interp, guard) < 0) { - if (guard != &warnings_guard) { - PyMem_RawFree(guard); - } - // Finalization has begun; this thread must stop before it takes - // the lock. - _PyThreadState_HangThread(_PyThreadState_GET()); + PyThreadState *tstate = _PyThreadState_GET(); + _PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate; + if (!_PyRecursiveMutex_IsLockedByCurrentThread(&st->lock) && + _PyInterpreterState_GetFinalizing(interp) != tstate) + { + assert(tstate_impl->warnings_lock_guard == NULL); + PyInterpreterGuard *guard = PyInterpreterGuard_FromCurrent(); + if (guard == NULL) { + return -1; } + tstate_impl->warnings_lock_guard = guard; } _PyRecursiveMutex_Lock(&st->lock); - st->lock_guard = guard; return 0; } @@ -302,17 +289,12 @@ warnings_unlock(PyInterpreterState *interp) _PyRecursiveMutex_Unlock(&st->lock); return 0; } - PyInterpreterGuard *guard = st->lock_guard; - st->lock_guard = NULL; + _PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET(); + PyInterpreterGuard *guard = tstate->warnings_lock_guard; + tstate->warnings_lock_guard = NULL; _PyRecursiveMutex_Unlock(&st->lock); if (guard != NULL) { - _PyInterpreterGuard_Release(guard); - if (guard == &warnings_guard) { - guard->interp = NULL; - } - else { - PyMem_RawFree(guard); - } + PyInterpreterGuard_Close(guard); } return 0; }