gh-117657: Make Py_TYPE and Py_SET_TYPE thread safe (GH-120165)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Nadeshiko Manju <me@manjusaka.me>

diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
index 86dada5061e7b569e87a7287b8da90feb6b0c421..6b5f50b88f7b85d0fe5d36ab42dd4abab9db3a24 100644 (file)
--- a/Include/internal/pycore_interp.h
+++ b/Include/internal/pycore_interp.h
@@ -401,7 +401,10 @@ PyAPI_FUNC(PyStatus) _PyInterpreterState_New(
 #define RARE_EVENT_INTERP_INC(interp, name) \
     do { \
         /* saturating add */ \
-        if (interp->rare_events.name < UINT8_MAX) interp->rare_events.name++; \
+        int val = FT_ATOMIC_LOAD_UINT8_RELAXED(interp->rare_events.name); \
+        if (val < UINT8_MAX) { \
+            FT_ATOMIC_STORE_UINT8(interp->rare_events.name, val + 1); \
+        } \
         RARE_EVENT_STAT_INC(name); \
     } while (0); \
 

diff --git a/Include/object.h b/Include/object.h
index c8c63b9b2b1450cdaec7309a711a6c98adda7da0..4a39ada8c7daa4327adbff598a7350a371823bb0 100644 (file)
--- a/Include/object.h
+++ b/Include/object.h
@@ -246,7 +246,11 @@ _Py_IsOwnedByCurrentThread(PyObject *ob)
 
 // bpo-39573: The Py_SET_TYPE() function must be used to set an object type.
 static inline PyTypeObject* Py_TYPE(PyObject *ob) {
+#ifdef Py_GIL_DISABLED
+    return (PyTypeObject *)_Py_atomic_load_ptr_relaxed(&ob->ob_type);
+#else
     return ob->ob_type;
+#endif
 }
 #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 < 0x030b0000
 #  define Py_TYPE(ob) Py_TYPE(_PyObject_CAST(ob))
@@ -274,7 +278,11 @@ static inline int Py_IS_TYPE(PyObject *ob, PyTypeObject *type) {
 
 
 static inline void Py_SET_TYPE(PyObject *ob, PyTypeObject *type) {
+#ifdef Py_GIL_DISABLED
+    _Py_atomic_store_ptr(&ob->ob_type, type);
+#else
     ob->ob_type = type;
+#endif
 }
 #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 < 0x030b0000
 #  define Py_SET_TYPE(ob, type) Py_SET_TYPE(_PyObject_CAST(ob), type)

diff --git a/Lib/test/test_free_threading/test_type.py b/Lib/test/test_free_threading/test_type.py
index 786336fa0cddce7763aea3a7f20a0bcf93862e0d..1e84b2db2d48827cf4e15c9bbca335917157de96 100644 (file)
--- a/Lib/test/test_free_threading/test_type.py
+++ b/Lib/test/test_free_threading/test_type.py
@@ -96,6 +96,32 @@ class TestType(TestCase):
 
         self.run_one(writer_func, reader_func)
 
+    def test___class___modification(self):
+        class Foo:
+            pass
+
+        class Bar:
+            pass
+
+        thing = Foo()
+        def work():
+            foo = thing
+            for _ in range(10000):
+                foo.__class__ = Bar
+                type(foo)
+                foo.__class__ = Foo
+                type(foo)
+
+
+        threads = []
+        for i in range(NTHREADS):
+            thread = threading.Thread(target=work)
+            thread.start()
+            threads.append(thread)
+
+        for thread in threads:
+            thread.join()
+
     def run_one(self, writer_func, reader_func):
         writer = Thread(target=writer_func)
         readers = []

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 070e3d2f7bf2b472055a2985cbef5f84317a3503..8ecab555454cdcf4c8dd05efb0337375d88e9968 100644 (file)
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -6633,9 +6633,15 @@ object_set_class(PyObject *self, PyObject *value, void *closure)
         if (newto->tp_flags & Py_TPFLAGS_HEAPTYPE) {
             Py_INCREF(newto);
         }
+        Py_BEGIN_CRITICAL_SECTION(self);
+        // The real Py_TYPE(self) (`oldto`) may have changed from
+        // underneath us in another thread, so we re-fetch it here.
+        oldto = Py_TYPE(self);
         Py_SET_TYPE(self, newto);
-        if (oldto->tp_flags & Py_TPFLAGS_HEAPTYPE)
+        Py_END_CRITICAL_SECTION();
+        if (oldto->tp_flags & Py_TPFLAGS_HEAPTYPE) {
             Py_DECREF(oldto);
+        }
 
         RARE_EVENT_INC(set_class);
         return 0;

diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt
index cb48a30751ac7ba4c8621916036a70c057b2c96a..b10b297f50da811141fe2502bb63c1d4ff233dad 100644 (file)
--- a/Tools/tsan/suppressions_free_threading.txt
+++ b/Tools/tsan/suppressions_free_threading.txt
@@ -37,7 +37,6 @@ race_top:set_contains_key
 # https://gist.github.com/colesbury/d13d033f413b4ad07929d044bed86c35
 race_top:set_discard_entry
 race_top:set_inheritable
-race_top:Py_SET_TYPE
 race_top:_PyDict_CheckConsistency
 race_top:_Py_dict_lookup_threadsafe
 race_top:_multiprocessing_SemLock_acquire_impl
@@ -58,7 +57,6 @@ race_top:_PyFrame_Initialize
 race_top:PyInterpreterState_ThreadHead
 race_top:_PyObject_TryGetInstanceAttribute
 race_top:PyThreadState_Next
-race_top:Py_TYPE
 race_top:PyUnstable_InterpreterFrame_GetLine
 race_top:sock_close
 race_top:tstate_delete_common