]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-119525: Fix deadlock with `_PyType_Lookup` and the GIL (#119527)
authorSam Gross <colesbury@gmail.com>
Wed, 29 May 2024 19:26:04 +0000 (15:26 -0400)
committerGitHub <noreply@github.com>
Wed, 29 May 2024 19:26:04 +0000 (15:26 -0400)
The deadlock only affected the free-threaded build and only occurred
when the GIL was enabled at runtime. The `Py_DECREF(old_name)` call
might temporarily release the GIL while holding the type seqlock.
Another thread may spin trying to acquire the seqlock while holding the
GIL.

The deadlock occurred roughly 1 in ~1,000 runs of `pool_in_threads.py`
from `test_multiprocessing_pool_circular_import`.

Misc/NEWS.d/next/Core and Builtins/2024-05-24-21-04-00.gh-issue-119525.zLFLf1.rst [new file with mode: 0644]
Objects/typeobject.c

diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-24-21-04-00.gh-issue-119525.zLFLf1.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-24-21-04-00.gh-issue-119525.zLFLf1.rst
new file mode 100644 (file)
index 0000000..83c29a1
--- /dev/null
@@ -0,0 +1,2 @@
+Fix deadlock involving ``_PyType_Lookup()`` cache in the free-threaded build
+when the GIL is dynamically enabled at runtime.
index 9d849d83082ccd2f7b8f1d32a1d00d7ed71c33b9..290306cdb677e52319de2a83839ec94d402ee3bb 100644 (file)
@@ -5169,7 +5169,7 @@ is_dunder_name(PyObject *name)
     return 0;
 }
 
-static void
+static PyObject *
 update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int version_tag, PyObject *value)
 {
     _Py_atomic_store_uint32_relaxed(&entry->version, version_tag);
@@ -5180,7 +5180,7 @@ update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int versio
     // exact unicode object or Py_None so it's safe to do so.
     PyObject *old_name = entry->name;
     _Py_atomic_store_ptr_relaxed(&entry->name, Py_NewRef(name));
-    Py_DECREF(old_name);
+    return old_name;
 }
 
 #if Py_GIL_DISABLED
@@ -5200,10 +5200,12 @@ update_cache_gil_disabled(struct type_cache_entry *entry, PyObject *name,
         return;
     }
 
-    update_cache(entry, name, version_tag, value);
+    PyObject *old_value = update_cache(entry, name, version_tag, value);
 
     // Then update sequence to the next valid value
     _PySeqLock_UnlockWrite(&entry->sequence);
+
+    Py_DECREF(old_value);
 }
 
 #endif
@@ -5315,7 +5317,8 @@ _PyType_LookupRef(PyTypeObject *type, PyObject *name)
 #if Py_GIL_DISABLED
         update_cache_gil_disabled(entry, name, version, res);
 #else
-        update_cache(entry, name, version, res);
+        PyObject *old_value = update_cache(entry, name, version, res);
+        Py_DECREF(old_value);
 #endif
     }
     return res;