From cd74ed0a71f57e96bcd6c2a996587c58ef5da82d Mon Sep 17 00:00:00 2001 From: Ken Jin Date: Fri, 12 Jul 2024 19:35:53 +0800 Subject: [PATCH] [3.13] gh-120198: Stop the world when setting __class__ on free-threaded build (#121591) (cherry-picked from commit 3bfc9c8) --- Include/internal/pycore_dict.h | 2 + Include/object.h | 8 -- Lib/test/test_free_threading/test_type.py | 2 +- Objects/dictobject.c | 15 ++-- Objects/typeobject.c | 96 ++++++++++++----------- 5 files changed, 65 insertions(+), 58 deletions(-) diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index 8d8d3748eda..01f80e21ba2 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -323,6 +323,8 @@ _PyInlineValuesSize(PyTypeObject *tp) int _PyDict_DetachFromObject(PyDictObject *dict, PyObject *obj); +PyDictObject *_PyObject_MaterializeManagedDict_LockHeld(PyObject *); + #ifdef __cplusplus } #endif diff --git a/Include/object.h b/Include/object.h index fa9c2a51a95..b4db7fb204f 100644 --- a/Include/object.h +++ b/Include/object.h @@ -327,11 +327,7 @@ 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,11 +417,7 @@ 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 1e84b2db2d4..29ca929994c 100644 --- a/Lib/test/test_free_threading/test_type.py +++ b/Lib/test/test_free_threading/test_type.py @@ -106,7 +106,7 @@ class TestType(TestCase): thing = Foo() def work(): foo = thing - for _ in range(10000): + for _ in range(5000): foo.__class__ = Bar type(foo) foo.__class__ = Foo diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 3cd26755634..48aed1e4da8 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -158,6 +158,10 @@ ASSERT_DICT_LOCKED(PyObject *op) if (!_PyInterpreterState_GET()->stoptheworld.world_stopped) { \ ASSERT_DICT_LOCKED(op); \ } +#define ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(op) \ + if (!_PyInterpreterState_GET()->stoptheworld.world_stopped) { \ + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); \ + } #define IS_DICT_SHARED(mp) _PyObject_GC_IS_SHARED(mp) #define SET_DICT_SHARED(mp) _PyObject_GC_SET_SHARED(mp) @@ -226,6 +230,7 @@ static inline void split_keys_entry_added(PyDictKeysObject *keys) #define ASSERT_DICT_LOCKED(op) #define ASSERT_WORLD_STOPPED_OR_DICT_LOCKED(op) +#define ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(op) #define LOCK_KEYS(keys) #define UNLOCK_KEYS(keys) #define ASSERT_KEYS_LOCKED(keys) @@ -6673,10 +6678,10 @@ make_dict_from_instance_attributes(PyInterpreterState *interp, return res; } -static PyDictObject * -materialize_managed_dict_lock_held(PyObject *obj) +PyDictObject * +_PyObject_MaterializeManagedDict_LockHeld(PyObject *obj) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj); + ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(obj); PyDictValues *values = _PyObject_InlineValues(obj); PyInterpreterState *interp = _PyInterpreterState_GET(); @@ -6705,7 +6710,7 @@ _PyObject_MaterializeManagedDict(PyObject *obj) goto exit; } #endif - dict = materialize_managed_dict_lock_held(obj); + dict = _PyObject_MaterializeManagedDict_LockHeld(obj); #ifdef Py_GIL_DISABLED exit: @@ -7138,7 +7143,7 @@ PyObject_ClearManagedDict(PyObject *obj) int _PyDict_DetachFromObject(PyDictObject *mp, PyObject *obj) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj); + ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(obj); assert(_PyObject_ManagedDictPointer(obj)->dict == mp); assert(_PyObject_InlineValuesConsistencyCheck(obj)); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index edcdc6e9c4f..1227f524a58 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -6378,28 +6378,11 @@ differs: return 0; } + + static int -object_set_class(PyObject *self, PyObject *value, void *closure) +object_set_class_world_stopped(PyObject *self, PyTypeObject *newto) { - - if (value == NULL) { - PyErr_SetString(PyExc_TypeError, - "can't delete __class__ attribute"); - return -1; - } - if (!PyType_Check(value)) { - PyErr_Format(PyExc_TypeError, - "__class__ must be set to a class, not '%s' object", - Py_TYPE(value)->tp_name); - return -1; - } - PyTypeObject *newto = (PyTypeObject *)value; - - if (PySys_Audit("object.__setattr__", "OsO", - self, "__class__", value) < 0) { - return -1; - } - PyTypeObject *oldto = Py_TYPE(self); /* In versions of CPython prior to 3.5, the code in @@ -6465,39 +6448,66 @@ object_set_class(PyObject *self, PyObject *value, void *closure) /* Changing the class will change the implicit dict keys, * so we must materialize the dictionary first. */ if (oldto->tp_flags & Py_TPFLAGS_INLINE_VALUES) { - PyDictObject *dict = _PyObject_MaterializeManagedDict(self); + PyDictObject *dict = _PyObject_GetManagedDict(self); if (dict == NULL) { - return -1; + dict = _PyObject_MaterializeManagedDict_LockHeld(self); + if (dict == NULL) { + return -1; + } } - bool error = false; - - Py_BEGIN_CRITICAL_SECTION2(self, dict); - - // If we raced after materialization and replaced the dict - // then the materialized dict should no longer have the - // inline values in which case detach is a nop. - assert(_PyObject_GetManagedDict(self) == dict || - dict->ma_values != _PyObject_InlineValues(self)); + assert(_PyObject_GetManagedDict(self) == dict); if (_PyDict_DetachFromObject(dict, self) < 0) { - error = true; - } - - Py_END_CRITICAL_SECTION2(); - if (error) { return -1; } + } 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); - Py_END_CRITICAL_SECTION(); + + return 0; + } + else { + return -1; + } +} + +static int +object_set_class(PyObject *self, PyObject *value, void *closure) +{ + + if (value == NULL) { + PyErr_SetString(PyExc_TypeError, + "can't delete __class__ attribute"); + return -1; + } + if (!PyType_Check(value)) { + PyErr_Format(PyExc_TypeError, + "__class__ must be set to a class, not '%s' object", + Py_TYPE(value)->tp_name); + return -1; + } + PyTypeObject *newto = (PyTypeObject *)value; + + if (PySys_Audit("object.__setattr__", "OsO", + self, "__class__", value) < 0) { + return -1; + } + +#ifdef Py_GIL_DISABLED + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); +#endif + PyTypeObject *oldto = Py_TYPE(self); + int res = object_set_class_world_stopped(self, newto); +#ifdef Py_GIL_DISABLED + _PyEval_StartTheWorld(interp); +#endif + if (res == 0) { if (oldto->tp_flags & Py_TPFLAGS_HEAPTYPE) { Py_DECREF(oldto); } @@ -6505,9 +6515,7 @@ object_set_class(PyObject *self, PyObject *value, void *closure) RARE_EVENT_INC(set_class); return 0; } - else { - return -1; - } + return res; } static PyGetSetDef object_getsets[] = {