Stowage/cpython
2
0
Fork 0
mirror of https://github.com/python/cpython.git synced 2025-12-08 06:10:17 +00:00
Code Issues Projects Releases Packages Wiki Activity

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

Browse source 
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).
This commit is contained in:
Neil Schemenauer 2025-04-28 15:08:09 -07:00 • committed by GitHub
parent b71442f44f
commit ca46ec85f8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 13 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Misc/NEWS.d/next/Core and Builtins/2025-04-26-17-50-47.gh-issue-132942.aEEZvZ.rst Normal file
View file

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

14
Objects/typeobject.c
View file

@ -5164,7 +5164,6 @@ is_dunder_name(PyObject *name)
static PyObject * static PyObject *
update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int version_tag, PyObject *value) 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 */ _Py_atomic_store_ptr_relaxed(&entry->value, value); /* borrowed */
assert(_PyASCIIObject_CAST(name)->hash != -1); assert(_PyASCIIObject_CAST(name)->hash != -1);
OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name); 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. // exact unicode object or Py_None so it's safe to do so.
PyObject *old_name = entry->name; PyObject *old_name = entry->name;
_Py_atomic_store_ptr_relaxed(&entry->name, Py_NewRef(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; 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 // synchronize-with other writing threads by doing an acquire load on the sequence
while (1) { while (1) {
uint32_t sequence = _PySeqLock_BeginRead(&entry->sequence); 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); uint32_t type_version = _Py_atomic_load_uint32_acquire(&type->tp_version_tag);
if (entry_version == type_version && if (entry_version == type_version &&
_Py_atomic_load_ptr_relaxed(&entry->name) == name) { _Py_atomic_load_ptr_relaxed(&entry->name) == name) {
@ -5281,11 +5286,14 @@ _PyType_LookupRef(PyTypeObject *type, PyObject *name)
int has_version = 0; int has_version = 0;
int version = 0; int version = 0;
BEGIN_TYPE_LOCK(); 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)) { if (MCACHE_CACHEABLE_NAME(name)) {
has_version = assign_version_tag(interp, type); has_version = assign_version_tag(interp, type);
version = type->tp_version_tag; version = type->tp_version_tag;
} }
res = find_name_in_mro(type, name, &error);
END_TYPE_LOCK(); END_TYPE_LOCK();
/* Only put NULL results into cache if there was no error. */ /* Only put NULL results into cache if there was no error. */