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 31d1342

Browse filesBrowse files
authored
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).
1 parent fe462f5 commit 31d1342
Copy full SHA for 31d1342

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
@@ -5678,14 +5678,19 @@ 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);
56825681
_Py_atomic_store_ptr_relaxed(&entry->value, value); /* borrowed */
56835682
assert(_PyASCIIObject_CAST(name)->hash != -1);
56845683
OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name);
56855684
// We're releasing this under the lock for simplicity sake because it's always a
56865685
// exact unicode object or Py_None so it's safe to do so.
56875686
PyObject *old_name = entry->name;
56885687
_Py_atomic_store_ptr_relaxed(&entry->name, Py_NewRef(name));
5688+
// We must write the version last to avoid _Py_TryXGetStackRef()
5689+
// operating on an invalid (already deallocated) value inside
5690+
// _PyType_LookupRefAndVersion(). If we write the version first then a
5691+
// reader could pass the "entry_version == type_version" check but could
5692+
// be using the old entry value.
5693+
_Py_atomic_store_uint32_release(&entry->version, version_tag);
56895694
return old_name;
56905695
}
56915696

@@ -5762,7 +5767,7 @@ _PyType_LookupStackRefAndVersion(PyTypeObject *type, PyObject *name, _PyStackRef
57625767
// synchronize-with other writing threads by doing an acquire load on the sequence
57635768
while (1) {
57645769
uint32_t sequence = _PySeqLock_BeginRead(&entry->sequence);
5765-
uint32_t entry_version = _Py_atomic_load_uint32_relaxed(&entry->version);
5770+
uint32_t entry_version = _Py_atomic_load_uint32_acquire(&entry->version);
57665771
uint32_t type_version = _Py_atomic_load_uint32_acquire(&type->tp_version_tag);
57675772
if (entry_version == type_version &&
57685773
_Py_atomic_load_ptr_relaxed(&entry->name) == name) {
@@ -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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.