From b6c62c79e7d9592ca1ea6b93f6ce3dd3829939d0 Mon Sep 17 00:00:00 2001 From: mpage Date: Wed, 17 Apr 2024 09:42:56 -0700 Subject: [PATCH] gh-117657: Fix data races in the method cache in free-threaded builds (#117954) Fix data races in the method cache in free-threaded builds These are technically data races, but I think they're benign (to the extent that that is actually possible). We update cache entries non-atomically but read them atomically from another thread, and there's nothing that establishes a happens-before relationship between the reads and writes that I can see. --- Objects/typeobject.c | 8 +++++--- Tools/tsan/suppressions_free_threading.txt | 1 - 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 1cb53516a9ae76..6304ee178fd038 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4974,13 +4974,15 @@ is_dunder_name(PyObject *name) static void update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int version_tag, PyObject *value) { - entry->version = version_tag; - entry->value = value; /* borrowed */ + _Py_atomic_store_uint32_relaxed(&entry->version, version_tag); + _Py_atomic_store_ptr_relaxed(&entry->value, value); /* borrowed */ assert(_PyASCIIObject_CAST(name)->hash != -1); OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name); // We're releasing this under the lock for simplicity sake because it's always a // exact unicode object or Py_None so it's safe to do so. - Py_SETREF(entry->name, Py_NewRef(name)); + PyObject *old_name = entry->name; + _Py_atomic_store_ptr_relaxed(&entry->name, Py_NewRef(name)); + Py_DECREF(old_name); } #if Py_GIL_DISABLED diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt index 889b62e59b14a6..6e2bdc1ea85cd6 100644 --- a/Tools/tsan/suppressions_free_threading.txt +++ b/Tools/tsan/suppressions_free_threading.txt @@ -47,5 +47,4 @@ race:set_inheritable race:start_the_world race:tstate_set_detached race:unicode_hash -race:update_cache race:update_cache_gil_disabled