gh-87135: threading.Lock: Raise rather than hang on Python finalization (GH-135991)

encukou · web-flow · commit fe119a08177f · 2025-07-01T10:57:42.000+02:00
After Python finalization gets to the point where no other thread
can attach thread state, attempting to acquire a Python lock must hang.
Raise PythonFinalizationError instead of hanging.
diff --git a/Doc/library/exceptions.rst b/Doc/library/exceptions.rst
@@ -429,7 +429,9 @@ The following exceptions are the exceptions that are usually raised.
 
    * Creating a new Python thread.
    * :meth:`Joining <threading.Thread.join>` a running daemon thread.
-   * :func:`os.fork`.
+   * :func:`os.fork`,
+   * acquiring a lock such as :class:`threading.Lock`, when it is known that
+     the operation would otherwise deadlock.
 
    See also the :func:`sys.is_finalizing` function.
 
@@ -440,6 +442,11 @@ The following exceptions are the exceptions that are usually raised.
 
       :meth:`threading.Thread.join` can now raise this exception.
 
+   .. versionchanged:: next
+
+      This exception may be raised when acquiring :meth:`threading.Lock`
+      or :meth:`threading.RLock`.
+
 .. exception:: RecursionError
 
    This exception is derived from :exc:`RuntimeError`.  It is raised when the
diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h
@@ -51,6 +51,11 @@ typedef enum _PyLockFlags {
 
     // Fail if interrupted by a signal while waiting on the lock.
     _PY_FAIL_IF_INTERRUPTED = 4,
+
+    // Locking & unlocking this lock requires attached thread state.
+    // If locking returns PY_LOCK_FAILURE, a Python exception *may* be raised.
+    // (Intended for use with _PY_LOCK_HANDLE_SIGNALS and _PY_LOCK_DETACH.)
+    _PY_LOCK_PYTHONLOCK = 8,
 } _PyLockFlags;
 
 // Lock a mutex with an optional timeout and additional options. See
diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
@@ -1247,6 +1247,61 @@ def __del__(self):
         self.assertEqual(err, b"")
         self.assertIn(b"all clear", out)
 
+    @support.subTests('lock_class_name', ['Lock', 'RLock'])
+    def test_acquire_daemon_thread_lock_in_finalization(self, lock_class_name):
+        # gh-123940: Py_Finalize() prevents other threads from running Python
+        # code (and so, releasing locks), so acquiring a locked lock can not
+        # succeed.
+        # We raise an exception rather than hang.
+        code = textwrap.dedent(f"""
+            import threading
+            import time
+
+            thread_started_event = threading.Event()
+
+            lock = threading.{lock_class_name}()
+            def loop():
+                if {lock_class_name!r} == 'RLock':
+                    lock.acquire()
+                with lock:
+                    thread_started_event.set()
+                    while True:
+                        time.sleep(1)
+
+            uncontested_lock = threading.{lock_class_name}()
+
+            class Cycle:
+                def __init__(self):
+                    self.self_ref = self
+                    self.thr = threading.Thread(
+                        target=loop, daemon=True)
+                    self.thr.start()
+                    thread_started_event.wait()
+
+                def __del__(self):
+                    assert self.thr.is_alive()
+
+                    # We *can* acquire an unlocked lock
+                    uncontested_lock.acquire()
+                    if {lock_class_name!r} == 'RLock':
+                        uncontested_lock.acquire()
+
+                    # Acquiring a locked one fails
+                    try:
+                        lock.acquire()
+                    except PythonFinalizationError:
+                        assert self.thr.is_alive()
+                        print('got the correct exception!')
+
+            # 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"")
+        self.assertIn(b"got the correct exception", out)
+
     def test_start_new_thread_failed(self):
         # gh-109746: if Python fails to start newly created thread
         # due to failure of underlying PyThread_start_new_thread() call,
diff --git a/Misc/NEWS.d/next/Library/2025-06-27-09-26-04.gh-issue-87135.33z0UW.rst b/Misc/NEWS.d/next/Library/2025-06-27-09-26-04.gh-issue-87135.33z0UW.rst
@@ -0,0 +1,3 @@
+Acquiring a :class:`threading.Lock` or :class:`threading.RLock` at interpreter
+shutdown will raise :exc:`PythonFinalizationError` if Python can determine
+that it would otherwise deadlock.
diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
@@ -834,9 +834,14 @@ lock_PyThread_acquire_lock(PyObject *op, PyObject *args, PyObject *kwds)
         return NULL;
     }
 
-    PyLockStatus r = _PyMutex_LockTimed(&self->lock, timeout,
-                                        _PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH);
+    PyLockStatus r = _PyMutex_LockTimed(
+        &self->lock, timeout,
+        _PY_LOCK_PYTHONLOCK | _PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH);
     if (r == PY_LOCK_INTR) {
+        assert(PyErr_Occurred());
+        return NULL;
+    }
+    if (r == PY_LOCK_FAILURE && PyErr_Occurred()) {
         return NULL;
     }
 
@@ -1054,9 +1059,14 @@ rlock_acquire(PyObject *op, PyObject *args, PyObject *kwds)
         return NULL;
     }
 
-    PyLockStatus r = _PyRecursiveMutex_LockTimed(&self->lock, timeout,
-                                                 _PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH);
+    PyLockStatus r = _PyRecursiveMutex_LockTimed(
+        &self->lock, timeout,
+        _PY_LOCK_PYTHONLOCK | _PY_LOCK_HANDLE_SIGNALS | _PY_LOCK_DETACH);
     if (r == PY_LOCK_INTR) {
+        assert(PyErr_Occurred());
+        return NULL;
+    }
+    if (r == PY_LOCK_FAILURE && PyErr_Occurred()) {
         return NULL;
     }
 
diff --git a/Python/lock.c b/Python/lock.c
@@ -95,6 +95,18 @@ _PyMutex_LockTimed(PyMutex *m, PyTime_t timeout, _PyLockFlags flags)
         if (timeout == 0) {
             return PY_LOCK_FAILURE;
         }
+        if ((flags & _PY_LOCK_PYTHONLOCK) && Py_IsFinalizing()) {
+            // At this phase of runtime shutdown, only the finalization thread
+            // can have attached thread state; others hang if they try
+            // attaching. And since operations on this lock requires attached
+            // thread state (_PY_LOCK_PYTHONLOCK), the finalization thread is
+            // running this code, and no other thread can unlock.
+            // Raise rather than hang. (_PY_LOCK_PYTHONLOCK allows raising
+            // exceptons.)
+            PyErr_SetString(PyExc_PythonFinalizationError,
+                            "cannot acquire lock at interpreter finalization");
+            return PY_LOCK_FAILURE;
+        }
 
         uint8_t newv = v;
         if (!(v & _Py_HAS_PARKED)) {

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	+Acquiring a :class:`threading.Lock` or :class:`threading.RLock` at interpreter
	`2`	+shutdown will raise :exc:`PythonFinalizationError` if Python can determine
	`3`	`+that it would otherwise deadlock.`