]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-117657: Fix data races in the method cache in free-threaded builds (#117954)
authormpage <mpage@meta.com>
Wed, 17 Apr 2024 16:42:56 +0000 (09:42 -0700)
committerGitHub <noreply@github.com>
Wed, 17 Apr 2024 16:42:56 +0000 (09:42 -0700)
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
Tools/tsan/suppressions_free_threading.txt

index 1cb53516a9ae76dc5048afd22a6cbffffc86a350..6304ee178fd038d8b95f77bfceeb07ee03a57b33 100644 (file)
@@ -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
index 889b62e59b14a609c397493075b91fd6e83cb04e..6e2bdc1ea85cd6cbf5867b52b35eac41b5feebf1 100644 (file)
@@ -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