Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit d621626

Browse filesBrowse files
committed
[3.13] 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 70f04ea commit d621626
Copy full SHA for d621626

File tree

2 files changed

+13
-3
lines changed
Filter options

2 files changed

+13
-3
lines changed
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix two races in the type lookup cache. This affected the free-threaded
2+
build and could cause crashes (apparently quite difficult to trigger).

‎Objects/typeobject.c

Copy file name to clipboardExpand all lines: Objects/typeobject.c
+11-3Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5163,14 +5163,19 @@ is_dunder_name(PyObject *name)
51635163
static PyObject *
51645164
update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int version_tag, PyObject *value)
51655165
{
5166-
_Py_atomic_store_uint32_relaxed(&entry->version, version_tag);
51675166
_Py_atomic_store_ptr_relaxed(&entry->value, value); /* borrowed */
51685167
assert(_PyASCIIObject_CAST(name)->hash != -1);
51695168
OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name);
51705169
// We're releasing this under the lock for simplicity sake because it's always a
51715170
// exact unicode object or Py_None so it's safe to do so.
51725171
PyObject *old_name = entry->name;
51735172
_Py_atomic_store_ptr_relaxed(&entry->name, Py_NewRef(name));
5173+
// We must write the version last to avoid _Py_TryXGetStackRef()
5174+
// operating on an invalid (already deallocated) value inside
5175+
// _PyType_LookupRefAndVersion(). If we write the version first then a
5176+
// reader could pass the "entry_version == type_version" check but could
5177+
// be using the old entry value.
5178+
_Py_atomic_store_uint32_release(&entry->version, version_tag);
51745179
return old_name;
51755180
}
51765181

@@ -5234,7 +5239,7 @@ _PyType_LookupRef(PyTypeObject *type, PyObject *name)
52345239
// synchronize-with other writing threads by doing an acquire load on the sequence
52355240
while (1) {
52365241
uint32_t sequence = _PySeqLock_BeginRead(&entry->sequence);
5237-
uint32_t entry_version = _Py_atomic_load_uint32_relaxed(&entry->version);
5242+
uint32_t entry_version = _Py_atomic_load_uint32_acquire(&entry->version);
52385243
uint32_t type_version = _Py_atomic_load_uint32_acquire(&type->tp_version_tag);
52395244
if (entry_version == type_version &&
52405245
_Py_atomic_load_ptr_relaxed(&entry->name) == name) {
@@ -5280,11 +5285,14 @@ _PyType_LookupRef(PyTypeObject *type, PyObject *name)
52805285
int has_version = 0;
52815286
int version = 0;
52825287
BEGIN_TYPE_LOCK();
5283-
res = find_name_in_mro(type, name, &error);
5288+
// We must assign the version before doing the lookup. If
5289+
// find_name_in_mro() blocks and releases the critical section
5290+
// then the type version can change.
52845291
if (MCACHE_CACHEABLE_NAME(name)) {
52855292
has_version = assign_version_tag(interp, type);
52865293
version = type->tp_version_tag;
52875294
}
5295+
res = find_name_in_mro(type, name, &error);
52885296
END_TYPE_LOCK();
52895297

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

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.