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 c551898

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 b71442f commit c551898
Copy full SHA for c551898

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
@@ -5164,14 +5164,19 @@ is_dunder_name(PyObject *name)
51645164
static PyObject *
51655165
update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int version_tag, PyObject *value)
51665166
{
5167-
_Py_atomic_store_uint32_relaxed(&entry->version, version_tag);
51685167
_Py_atomic_store_ptr_relaxed(&entry->value, value); /* borrowed */
51695168
assert(_PyASCIIObject_CAST(name)->hash != -1);
51705169
OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name);
51715170
// We're releasing this under the lock for simplicity sake because it's always a
51725171
// exact unicode object or Py_None so it's safe to do so.
51735172
PyObject *old_name = entry->name;
51745173
_Py_atomic_store_ptr_relaxed(&entry->name, Py_NewRef(name));
5174+
// We must write the version last to avoid _Py_TryXGetStackRef()
5175+
// operating on an invalid (already deallocated) value inside
5176+
// _PyType_LookupRefAndVersion(). If we write the version first then a
5177+
// reader could pass the "entry_version == type_version" check but could
5178+
// be using the old entry value.
5179+
_Py_atomic_store_uint32_release(&entry->version, version_tag);
51755180
return old_name;
51765181
}
51775182

@@ -5235,7 +5240,7 @@ _PyType_LookupRef(PyTypeObject *type, PyObject *name)
52355240
// synchronize-with other writing threads by doing an acquire load on the sequence
52365241
while (1) {
52375242
uint32_t sequence = _PySeqLock_BeginRead(&entry->sequence);
5238-
uint32_t entry_version = _Py_atomic_load_uint32_relaxed(&entry->version);
5243+
uint32_t entry_version = _Py_atomic_load_uint32_acquire(&entry->version);
52395244
uint32_t type_version = _Py_atomic_load_uint32_acquire(&type->tp_version_tag);
52405245
if (entry_version == type_version &&
52415246
_Py_atomic_load_ptr_relaxed(&entry->name) == name) {
@@ -5281,11 +5286,14 @@ _PyType_LookupRef(PyTypeObject *type, PyObject *name)
52815286
int has_version = 0;
52825287
int version = 0;
52835288
BEGIN_TYPE_LOCK();
5284-
res = find_name_in_mro(type, name, &error);
5289+
// We must assign the version before doing the lookup. If
5290+
// find_name_in_mro() blocks and releases the critical section
5291+
// then the type version can change.
52855292
if (MCACHE_CACHEABLE_NAME(name)) {
52865293
has_version = assign_version_tag(interp, type);
52875294
version = type->tp_version_tag;
52885295
}
5296+
res = find_name_in_mro(type, name, &error);
52895297
END_TYPE_LOCK();
52905298

52915299
/* 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.