Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-120198: Stop the world when setting __class__ on free-threaded build #120672

Merged
merged 8 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Include/internal/pycore_dict.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,8 @@ _PyInlineValuesSize(PyTypeObject *tp)
int
_PyDict_DetachFromObject(PyDictObject *dict, PyObject *obj);

PyDictObject *_PyObject_MaterializeManagedDict_LockHeld(PyObject *);

#ifdef __cplusplus
}
#endif
Expand Down
8 changes: 0 additions & 8 deletions Include/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,7 @@ PyAPI_FUNC(PyTypeObject*) Py_TYPE(PyObject *ob);
#else
static inline PyTypeObject* _Py_TYPE(PyObject *ob)
{
#if defined(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))
Expand Down Expand Up @@ -288,11 +284,7 @@ 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)
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_free_threading/test_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class Bar:
thing = Foo()
def work():
foo = thing
for _ in range(10000):
for _ in range(5000):
foo.__class__ = Bar
type(foo)
foo.__class__ = Foo
Expand Down
15 changes: 10 additions & 5 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -227,6 +231,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)
Expand Down Expand Up @@ -6703,10 +6708,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();
Expand Down Expand Up @@ -6735,7 +6740,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:
Expand Down Expand Up @@ -7170,7 +7175,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));

Expand Down
96 changes: 52 additions & 44 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -6604,28 +6604,11 @@ compatible_for_assignment(PyTypeObject* oldto, PyTypeObject* newto, const char*
return 0;
}

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;
}

static int
object_set_class_world_stopped(PyObject *self, PyTypeObject *newto)
{
PyTypeObject *oldto = Py_TYPE(self);

/* In versions of CPython prior to 3.5, the code in
Expand Down Expand Up @@ -6691,49 +6674,74 @@ 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);
}

RARE_EVENT_INC(set_class);
return 0;
}
else {
return -1;
}
return res;
}

static PyGetSetDef object_getsets[] = {
Expand Down
Loading