https://github.com/python/cpython/commit/ca46ec85f8c3cc5a9ae098c5b8fce435ba13c30e
commit: ca46ec85f8c3cc5a9ae098c5b8fce435ba13c30e
branch: 3.13
author: Neil Schemenauer <nas-git...@arctrix.com>
committer: nascheme <nas-git...@arctrix.com>
date: 2025-04-28T22:08:09Z
summary:

[3.13] gh-132942: Fix races in type lookup cache (gh-133114)

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

files:
A Misc/NEWS.d/next/Core and 
Builtins/2025-04-26-17-50-47.gh-issue-132942.aEEZvZ.rst
M Objects/typeobject.c

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
index 00000000000000..9b7cf5516182b2
--- /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 362ed1772e24f0..2b63c1bbfab337 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -5164,7 +5164,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);
@@ -5172,6 +5171,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;
 }
 
@@ -5235,7 +5240,7 @@ _PyType_LookupRef(PyTypeObject *type, PyObject *name)
     // 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) {
@@ -5281,11 +5286,14 @@ _PyType_LookupRef(PyTypeObject *type, PyObject *name)
     int has_version = 0;
     int 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);
         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. */

_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com

Reply via email to