gh-132942: Fix races in type lookup cache (gh-133032)

Two races related to the type lookup cache, when used in the
free-threaded build.  This caused test_opcache to sometimes fail (as
well as other hard to re-produce failures).

diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-04-26-17-50-47.gh-issue-132942.aEEZvZ.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-26-17-50-47.gh-issue-132942.aEEZvZ.rst
new file mode 100644 (file)
index 0000000..9b7cf55
--- /dev/null
+++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-26-17-50-47.gh-issue-132942.aEEZvZ.rst
@@ -0,0 +1,2 @@
+Fix two races in the type lookup cache.  This affected the free-threaded
+build and could cause crashes (apparently quite difficult to trigger).

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index a720a98121dc93aab2ea2016e453ebc3d34d0a1b..4e614daaa6955bd5a8d530cb4ea3c987a7e0f85e 100644 (file)
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -5678,7 +5678,6 @@ is_dunder_name(PyObject *name)
 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);
     _Py_atomic_store_ptr_relaxed(&entry->value, value); /* borrowed */
     assert(_PyASCIIObject_CAST(name)->hash != -1);
     OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name);
@@ -5686,6 +5685,12 @@ 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));
+    // We must write the version last to avoid _Py_TryXGetStackRef()
+    // operating on an invalid (already deallocated) value inside
+    // _PyType_LookupRefAndVersion().  If we write the version first then a
+    // reader could pass the "entry_version == type_version" check but could
+    // be using the old entry value.
+    _Py_atomic_store_uint32_release(&entry->version, version_tag);
     return old_name;
 }
 
@@ -5762,7 +5767,7 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef
     // synchronize-with other writing threads by doing an acquire load on the sequence
     while (1) {
         uint32_t sequence = _PySeqLock_BeginRead(&entry->sequence);
-        uint32_t entry_version = _Py_atomic_load_uint32_relaxed(&entry->version);
+        uint32_t entry_version = _Py_atomic_load_uint32_acquire(&entry->version);
         uint32_t type_version = _Py_atomic_load_uint32_acquire(&type->tp_version_tag);
         if (entry_version == type_version &&
             _Py_atomic_load_ptr_relaxed(&entry->name) == name) {
@@ -5809,11 +5814,14 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef
     int has_version = 0;
     unsigned int assigned_version = 0;
     BEGIN_TYPE_LOCK();
-    res = find_name_in_mro(type, name, &error);
+    // We must assign the version before doing the lookup.  If
+    // find_name_in_mro() blocks and releases the critical section
+    // then the type version can change.
     if (MCACHE_CACHEABLE_NAME(name)) {
         has_version = assign_version_tag(interp, type);
         assigned_version = type->tp_version_tag;
     }
+    res = find_name_in_mro(type, name, &error);
     END_TYPE_LOCK();
 
     /* Only put NULL results into cache if there was no error. */