gh-159485: Hold an interpreter guard while using the warnings lock

Author: ngoldbaum

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 {

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);

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);

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):

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.

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},

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,27 +255,66 @@ 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
 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, &registry))
         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:

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)
 {

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);