Skip to content

Commit 90a7d35

Browse files
committed
gh-132942: Fix races in type lookup cache
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).
1 parent b1fc8b6 commit 90a7d35

File tree

1 file changed

+10
-2
lines changed

1 file changed

+10
-2
lines changed

Objects/typeobject.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5678,8 +5678,13 @@ is_dunder_name(PyObject *name)
56785678
static PyObject *
56795679
update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int version_tag, PyObject *value)
56805680
{
5681-
_Py_atomic_store_uint32_relaxed(&entry->version, version_tag);
5681+
// We must write the value first to avoid _Py_TryXGetStackRef()
5682+
// operating on an invalid (already deallocated) value inside
5683+
// _PyType_LookupRefAndVersion(). If we write the version first then a
5684+
// reader could pass the "entry_version == type_version" check but could
5685+
// be using the old entry value.
56825686
_Py_atomic_store_ptr_relaxed(&entry->value, value); /* borrowed */
5687+
_Py_atomic_store_uint32_relaxed(&entry->version, version_tag);
56835688
assert(_PyASCIIObject_CAST(name)->hash != -1);
56845689
OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name);
56855690
// We're releasing this under the lock for simplicity sake because it's always a
@@ -5809,11 +5814,14 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef
58095814
int has_version = 0;
58105815
unsigned int assigned_version = 0;
58115816
BEGIN_TYPE_LOCK();
5812-
res = find_name_in_mro(type, name, &error);
5817+
// We must assign the version before doing the lookup. If
5818+
// find_name_in_mro() blocks and releases the critical section
5819+
// then the type version can change.
58135820
if (MCACHE_CACHEABLE_NAME(name)) {
58145821
has_version = assign_version_tag(interp, type);
58155822
assigned_version = type->tp_version_tag;
58165823
}
5824+
res = find_name_in_mro(type, name, &error);
58175825
END_TYPE_LOCK();
58185826

58195827
/* Only put NULL results into cache if there was no error. */

0 commit comments

Comments
 (0)