From 758178407de4d6f66ddfd1a3a580f8c480b27554 Mon Sep 17 00:00:00 2001
From: Matt Page <mpage@meta.com>
Date: Wed, 24 Apr 2024 15:26:06 -0700
Subject: [PATCH 1/2] Fix race data race in `_Py_IsOwnedByCurrentThread()`
We need to load `ob_tid` atomically; it may be set to zero in another
thread without any intervening operations that establish a happens-before
relationship.
Using a relaxed load should be sufficient; we do not depend on the store
publishing any other information.
Examples:
The load in `_Py_IsOwnedByCurrentThread()`:
https://github.com/python/cpython/blob/4b10e209c76f9f36f8ae2e4d713b3a01591c1856/Include/object.h#L306
races with the store in `_Py_ExplicitMergeRefcount()` ([example](https://gist.github.com/mpage/0342ebce4b5b446689c41cdd72f16e61)):
https://github.com/python/cpython/blob/4b10e209c76f9f36f8ae2e4d713b3a01591c1856/Objects/object.c#L421
and the store in `_Py_MergeZeroLocalRefcount()` ([example](https://gist.github.com/mpage/eaec11eda6e2d988ed0dd2bcfd6d606b)):
https://github.com/python/cpython/blob/4b10e209c76f9f36f8ae2e4d713b3a01591c1856/Objects/object.c#L378
---
Include/object.h | 2 +-
Tools/tsan/suppressions_free_threading.txt | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/Include/object.h b/Include/object.h
index ffcacf1a3ef4ed..52317d397dd534 100644
--- a/Include/object.h
+++ b/Include/object.h
@@ -303,7 +303,7 @@ _Py_ThreadId(void)
static inline Py_ALWAYS_INLINE int
_Py_IsOwnedByCurrentThread(PyObject *ob)
{
- return ob->ob_tid == _Py_ThreadId();
+ return _Py_atomic_load_uintptr_relaxed(&ob->ob_tid) == _Py_ThreadId();
}
#endif
diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt
index 1408103ba80f96..94be478dc75044 100644
--- a/Tools/tsan/suppressions_free_threading.txt
+++ b/Tools/tsan/suppressions_free_threading.txt
@@ -15,7 +15,6 @@ race:_add_to_weak_set
race:_in_weak_set
race:_mi_heap_delayed_free_partial
race:_Py_IsImmortal
-race:_Py_IsOwnedByCurrentThread
race:_PyEval_EvalFrameDefault
race:_PyFunction_SetVersion
race:_PyImport_AcquireLock
From 425aa3f4bc17bbc52cad0924442999c9ec811f57 Mon Sep 17 00:00:00 2001
From: Matt Page <mpage@meta.com>
Date: Thu, 25 Apr 2024 13:10:40 -0700
Subject: [PATCH 2/2] Only use atomics with TSAN
---
Include/object.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Include/object.h b/Include/object.h
index 52317d397dd534..505d31629104a9 100644
--- a/Include/object.h
+++ b/Include/object.h
@@ -303,7 +303,11 @@ _Py_ThreadId(void)
static inline Py_ALWAYS_INLINE int
_Py_IsOwnedByCurrentThread(PyObject *ob)
{
+#ifdef _Py_THREAD_SANITIZER
return _Py_atomic_load_uintptr_relaxed(&ob->ob_tid) == _Py_ThreadId();
+#else
+ return ob->ob_tid == _Py_ThreadId();
+#endif
}
#endif