Skip to content

Commit 3bfc9c8

Browse files
gh-120198: Stop the world when setting __class__ on free-threaded build (GH-120672)
1 parent a802277 commit 3bfc9c8

File tree

5 files changed

+65
-58
lines changed

5 files changed

+65
-58
lines changed

Include/internal/pycore_dict.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,8 @@ _PyInlineValuesSize(PyTypeObject *tp)
322322
int
323323
_PyDict_DetachFromObject(PyDictObject *dict, PyObject *obj);
324324

325+
PyDictObject *_PyObject_MaterializeManagedDict_LockHeld(PyObject *);
326+
325327
#ifdef __cplusplus
326328
}
327329
#endif

Include/object.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,7 @@ PyAPI_FUNC(PyTypeObject*) Py_TYPE(PyObject *ob);
249249
#else
250250
static inline PyTypeObject* _Py_TYPE(PyObject *ob)
251251
{
252-
#if defined(Py_GIL_DISABLED)
253-
return (PyTypeObject *)_Py_atomic_load_ptr_relaxed(&ob->ob_type);
254-
#else
255252
return ob->ob_type;
256-
#endif
257253
}
258254
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 < 0x030b0000
259255
# define Py_TYPE(ob) _Py_TYPE(_PyObject_CAST(ob))
@@ -284,11 +280,7 @@ static inline int Py_IS_TYPE(PyObject *ob, PyTypeObject *type) {
284280

285281

286282
static inline void Py_SET_TYPE(PyObject *ob, PyTypeObject *type) {
287-
#ifdef Py_GIL_DISABLED
288-
_Py_atomic_store_ptr(&ob->ob_type, type);
289-
#else
290283
ob->ob_type = type;
291-
#endif
292284
}
293285
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 < 0x030b0000
294286
# define Py_SET_TYPE(ob, type) Py_SET_TYPE(_PyObject_CAST(ob), type)

Lib/test/test_free_threading/test_type.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ class Bar:
106106
thing = Foo()
107107
def work():
108108
foo = thing
109-
for _ in range(10000):
109+
for _ in range(5000):
110110
foo.__class__ = Bar
111111
type(foo)
112112
foo.__class__ = Foo

Objects/dictobject.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,10 @@ ASSERT_DICT_LOCKED(PyObject *op)
158158
if (!_PyInterpreterState_GET()->stoptheworld.world_stopped) { \
159159
ASSERT_DICT_LOCKED(op); \
160160
}
161+
#define ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(op) \
162+
if (!_PyInterpreterState_GET()->stoptheworld.world_stopped) { \
163+
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op); \
164+
}
161165

162166
#define IS_DICT_SHARED(mp) _PyObject_GC_IS_SHARED(mp)
163167
#define SET_DICT_SHARED(mp) _PyObject_GC_SET_SHARED(mp)
@@ -227,6 +231,7 @@ static inline void split_keys_entry_added(PyDictKeysObject *keys)
227231

228232
#define ASSERT_DICT_LOCKED(op)
229233
#define ASSERT_WORLD_STOPPED_OR_DICT_LOCKED(op)
234+
#define ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(op)
230235
#define LOCK_KEYS(keys)
231236
#define UNLOCK_KEYS(keys)
232237
#define ASSERT_KEYS_LOCKED(keys)
@@ -6667,10 +6672,10 @@ make_dict_from_instance_attributes(PyInterpreterState *interp,
66676672
return res;
66686673
}
66696674

6670-
static PyDictObject *
6671-
materialize_managed_dict_lock_held(PyObject *obj)
6675+
PyDictObject *
6676+
_PyObject_MaterializeManagedDict_LockHeld(PyObject *obj)
66726677
{
6673-
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj);
6678+
ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(obj);
66746679

66756680
PyDictValues *values = _PyObject_InlineValues(obj);
66766681
PyInterpreterState *interp = _PyInterpreterState_GET();
@@ -6699,7 +6704,7 @@ _PyObject_MaterializeManagedDict(PyObject *obj)
66996704
goto exit;
67006705
}
67016706
#endif
6702-
dict = materialize_managed_dict_lock_held(obj);
6707+
dict = _PyObject_MaterializeManagedDict_LockHeld(obj);
67036708

67046709
#ifdef Py_GIL_DISABLED
67056710
exit:
@@ -7132,7 +7137,7 @@ PyObject_ClearManagedDict(PyObject *obj)
71327137
int
71337138
_PyDict_DetachFromObject(PyDictObject *mp, PyObject *obj)
71347139
{
7135-
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj);
7140+
ASSERT_WORLD_STOPPED_OR_OBJ_LOCKED(obj);
71367141
assert(_PyObject_ManagedDictPointer(obj)->dict == mp);
71377142
assert(_PyObject_InlineValuesConsistencyCheck(obj));
71387143

Objects/typeobject.c

Lines changed: 52 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -6540,28 +6540,11 @@ compatible_for_assignment(PyTypeObject* oldto, PyTypeObject* newto, const char*
65406540
return 0;
65416541
}
65426542

6543-
static int
6544-
object_set_class(PyObject *self, PyObject *value, void *closure)
6545-
{
6546-
6547-
if (value == NULL) {
6548-
PyErr_SetString(PyExc_TypeError,
6549-
"can't delete __class__ attribute");
6550-
return -1;
6551-
}
6552-
if (!PyType_Check(value)) {
6553-
PyErr_Format(PyExc_TypeError,
6554-
"__class__ must be set to a class, not '%s' object",
6555-
Py_TYPE(value)->tp_name);
6556-
return -1;
6557-
}
6558-
PyTypeObject *newto = (PyTypeObject *)value;
65596543

6560-
if (PySys_Audit("object.__setattr__", "OsO",
6561-
self, "__class__", value) < 0) {
6562-
return -1;
6563-
}
65646544

6545+
static int
6546+
object_set_class_world_stopped(PyObject *self, PyTypeObject *newto)
6547+
{
65656548
PyTypeObject *oldto = Py_TYPE(self);
65666549

65676550
/* In versions of CPython prior to 3.5, the code in
@@ -6627,49 +6610,74 @@ object_set_class(PyObject *self, PyObject *value, void *closure)
66276610
/* Changing the class will change the implicit dict keys,
66286611
* so we must materialize the dictionary first. */
66296612
if (oldto->tp_flags & Py_TPFLAGS_INLINE_VALUES) {
6630-
PyDictObject *dict = _PyObject_MaterializeManagedDict(self);
6613+
PyDictObject *dict = _PyObject_GetManagedDict(self);
66316614
if (dict == NULL) {
6632-
return -1;
6615+
dict = _PyObject_MaterializeManagedDict_LockHeld(self);
6616+
if (dict == NULL) {
6617+
return -1;
6618+
}
66336619
}
66346620

6635-
bool error = false;
6636-
6637-
Py_BEGIN_CRITICAL_SECTION2(self, dict);
6638-
6639-
// If we raced after materialization and replaced the dict
6640-
// then the materialized dict should no longer have the
6641-
// inline values in which case detach is a nop.
6642-
assert(_PyObject_GetManagedDict(self) == dict ||
6643-
dict->ma_values != _PyObject_InlineValues(self));
6621+
assert(_PyObject_GetManagedDict(self) == dict);
66446622

66456623
if (_PyDict_DetachFromObject(dict, self) < 0) {
6646-
error = true;
6647-
}
6648-
6649-
Py_END_CRITICAL_SECTION2();
6650-
if (error) {
66516624
return -1;
66526625
}
6626+
66536627
}
66546628
if (newto->tp_flags & Py_TPFLAGS_HEAPTYPE) {
66556629
Py_INCREF(newto);
66566630
}
6657-
Py_BEGIN_CRITICAL_SECTION(self);
6658-
// The real Py_TYPE(self) (`oldto`) may have changed from
6659-
// underneath us in another thread, so we re-fetch it here.
6660-
oldto = Py_TYPE(self);
6631+
66616632
Py_SET_TYPE(self, newto);
6662-
Py_END_CRITICAL_SECTION();
6633+
6634+
return 0;
6635+
}
6636+
else {
6637+
return -1;
6638+
}
6639+
}
6640+
6641+
static int
6642+
object_set_class(PyObject *self, PyObject *value, void *closure)
6643+
{
6644+
6645+
if (value == NULL) {
6646+
PyErr_SetString(PyExc_TypeError,
6647+
"can't delete __class__ attribute");
6648+
return -1;
6649+
}
6650+
if (!PyType_Check(value)) {
6651+
PyErr_Format(PyExc_TypeError,
6652+
"__class__ must be set to a class, not '%s' object",
6653+
Py_TYPE(value)->tp_name);
6654+
return -1;
6655+
}
6656+
PyTypeObject *newto = (PyTypeObject *)value;
6657+
6658+
if (PySys_Audit("object.__setattr__", "OsO",
6659+
self, "__class__", value) < 0) {
6660+
return -1;
6661+
}
6662+
6663+
#ifdef Py_GIL_DISABLED
6664+
PyInterpreterState *interp = _PyInterpreterState_GET();
6665+
_PyEval_StopTheWorld(interp);
6666+
#endif
6667+
PyTypeObject *oldto = Py_TYPE(self);
6668+
int res = object_set_class_world_stopped(self, newto);
6669+
#ifdef Py_GIL_DISABLED
6670+
_PyEval_StartTheWorld(interp);
6671+
#endif
6672+
if (res == 0) {
66636673
if (oldto->tp_flags & Py_TPFLAGS_HEAPTYPE) {
66646674
Py_DECREF(oldto);
66656675
}
66666676

66676677
RARE_EVENT_INC(set_class);
66686678
return 0;
66696679
}
6670-
else {
6671-
return -1;
6672-
}
6680+
return res;
66736681
}
66746682

66756683
static PyGetSetDef object_getsets[] = {

0 commit comments

Comments
 (0)