https://github.com/python/cpython/commit/31d1342de9489f95384dbc748130c2ae6f092e84
commit: 31d1342de9489f95384dbc748130c2ae6f092e84
branch: main
author: Neil Schemenauer <nas-git...@arctrix.com>
committer: nascheme <nas-git...@arctrix.com>
date: 2025-04-28T11:52:08-07:00
summary:
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).
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 a720a98121dc93..4e614daaa6955b 100644
--- 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. */
_______________________________________________
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
