From 91c4444d22a36119c83c9a21bfe0efe39d745086 Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Wed, 12 Jun 2024 15:37:26 +0200 Subject: [PATCH] [3.13] gh-117657: Make Py_TYPE and Py_SET_TYPE thread safe (GH-120165) (GH-120403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gh-117657: Make Py_TYPE and Py_SET_TYPE thread safe (GH-120165) (cherry picked from commit e16aed63f64b18a26859eff3de976ded373e66b8) Co-authored-by: Ken Jin Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Co-authored-by: Nadeshiko Manju --- Include/internal/pycore_interp.h | 5 ++++- Include/object.h | 8 +++++++ Lib/test/test_free_threading/test_type.py | 26 ++++++++++++++++++++++ Objects/typeobject.c | 8 ++++++- Tools/tsan/suppressions_free_threading.txt | 2 -- 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 86dada5061e7b5..6b5f50b88f7b85 100644 --- 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 9132784628a501..a687bf2f7cdd74 100644 --- a/Include/object.h +++ b/Include/object.h @@ -331,7 +331,11 @@ static inline Py_ssize_t Py_REFCNT(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)) @@ -421,7 +425,11 @@ static inline void Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt) { 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 786336fa0cddce..1e84b2db2d4882 100644 --- a/Lib/test/test_free_threading/test_type.py +++ b/Lib/test/test_free_threading/test_type.py @@ -96,6 +96,32 @@ def reader_func(): 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 2d001b76e09775..eec25b0afaa86d 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -6474,9 +6474,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 cb48a30751ac7b..b10b297f50da81 100644 --- 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