]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-87135: threading.Lock: Raise rather than hang on Python finalization (GH-135991)
authorPetr Viktorin <encukou@gmail.com>
Tue, 1 Jul 2025 08:57:42 +0000 (10:57 +0200)
committerGitHub <noreply@github.com>
Tue, 1 Jul 2025 08:57:42 +0000 (10:57 +0200)
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.

Doc/library/exceptions.rst
Include/internal/pycore_lock.h
Lib/test/test_threading.py
Misc/NEWS.d/next/Library/2025-06-27-09-26-04.gh-issue-87135.33z0UW.rst [new file with mode: 0644]
Modules/_threadmodule.c
Python/lock.c

index 9806ae80905ca0e4c4d669a191043a82d4123e69..c09e1615a5b733538e5487a826af9dac75ff397e 100644 (file)
@@ -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
index 32b60cc33a21f1f253c7876d3fb63fae16f86202..bd6011b60ac09a841beabe14af9cc8a283791cbc 100644 (file)
@@ -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
index 13b55d0f0a2e73f10d73b02788a2df7023e4ba61..00a3037c3e1e01dd19a15ea290bd8238ea5b9802 100644 (file)
@@ -1247,6 +1247,61 @@ class ThreadTests(BaseTestCase):
         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
new file mode 100644 (file)
index 0000000..4b6bc74
--- /dev/null
@@ -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.
index b6ccca943f2085b37a9a02e29e8dab746ae81a04..8886a9d6bd0c8d60939204b27dad129e84a01ba8 100644 (file)
@@ -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;
     }
 
index ea6ac00bfeccb4e3447f20e61d456278c56f5210..eb09019e0a236d8e8fdcb06b85b80b617728ff15 100644 (file)
@@ -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)) {