Skip to content

Commit d4ce112

Browse files
committed
Use stop-the-world for tp_flag changes too.
1 parent b173f17 commit d4ce112

File tree

1 file changed

+31
-14
lines changed

1 file changed

+31
-14
lines changed

Objects/typeobject.c

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ class object "PyObject *" "&PyBaseObject_Type"
8484

8585
#endif
8686

87+
// Modification of type slots and type flags is protected by the global type
88+
// lock. However, the slots and flags are read non-atomically without holding
89+
// the type lock. So, we need to stop-the-world while modifying these, in
90+
// order to avoid data races. This is unfortunately a bit expensive.
91+
#ifdef Py_GIL_DISABLED
92+
// If defined, type slot updates (after initial creation) will stop-the-world.
93+
#define TYPE_SLOT_UPDATE_NEEDS_STOP
94+
// If defined, type flag updates (after initial creation) will stop-the-world.
95+
#define TYPE_FLAGS_UPDATE_NEEDS_STOP
96+
#endif
97+
8798
#define PyTypeObject_CAST(op) ((PyTypeObject *)(op))
8899

89100
typedef struct PySlot_Offset {
@@ -1582,9 +1593,10 @@ type_set_abstractmethods(PyObject *tp, PyObject *value, void *Py_UNUSED(closure)
15821593
BEGIN_TYPE_LOCK();
15831594
type_modified_unlocked(type);
15841595
if (abstract)
1585-
type_add_flags(type, Py_TPFLAGS_IS_ABSTRACT);
1596+
_PyType_SetFlags(type, 0, Py_TPFLAGS_IS_ABSTRACT);
15861597
else
1587-
type_clear_flags(type, Py_TPFLAGS_IS_ABSTRACT);
1598+
_PyType_SetFlags(type, Py_TPFLAGS_IS_ABSTRACT, 0);
1599+
15881600
END_TYPE_LOCK();
15891601

15901602
return 0;
@@ -5780,7 +5792,17 @@ void
57805792
_PyType_SetFlags(PyTypeObject *self, unsigned long mask, unsigned long flags)
57815793
{
57825794
BEGIN_TYPE_LOCK();
5783-
type_set_flags_with_mask(self, mask, flags);
5795+
unsigned long new_flags = (self->tp_flags & ~mask) | flags;
5796+
if (new_flags != self->tp_flags) {
5797+
#ifdef TYPE_FLAGS_UPDATE_NEEDS_STOP
5798+
PyInterpreterState *interp = _PyInterpreterState_GET();
5799+
_PyEval_StopTheWorld(interp);
5800+
#endif
5801+
self->tp_flags = new_flags;
5802+
#ifdef TYPE_FLAGS_UPDATE_NEEDS_STOP
5803+
_PyEval_StartTheWorld(interp);
5804+
#endif
5805+
}
57845806
END_TYPE_LOCK();
57855807
}
57865808

@@ -5944,14 +5966,10 @@ _Py_type_getattro(PyObject *tp, PyObject *name)
59445966
return _Py_type_getattro_impl(type, name, NULL);
59455967
}
59465968

5947-
#ifdef Py_GIL_DISABLED
5969+
#ifdef TYPE_SLOT_UPDATE_NEEDS_STOP
59485970
static int
59495971
update_slot_world_stopped(PyTypeObject *type, PyObject *name)
59505972
{
5951-
// Modification of type slots is protected by the global type
5952-
// lock. However, type slots are read non-atomically without holding the
5953-
// type lock. So, we need to stop-the-world while modifying slots, in
5954-
// order to avoid data races. This is unfortunately quite expensive.
59555973
int ret;
59565974
PyInterpreterState *interp = _PyInterpreterState_GET();
59575975
_PyEval_StopTheWorld(interp);
@@ -5990,7 +6008,7 @@ type_update_dict(PyTypeObject *type, PyDictObject *dict, PyObject *name,
59906008
return -1;
59916009
}
59926010

5993-
#if Py_GIL_DISABLED
6011+
#ifdef TYPE_SLOT_UPDATE_NEEDS_STOP
59946012
if (is_dunder_name(name) && has_slotdef(name)) {
59956013
return update_slot_world_stopped(type, name);
59966014
}
@@ -11026,10 +11044,9 @@ resolve_slotdups(PyTypeObject *type, PyObject *name)
1102611044
#undef ptrs
1102711045
}
1102811046

11029-
#ifdef Py_GIL_DISABLED
11047+
#ifdef TYPE_SLOT_UPDATE_NEEDS_STOP
1103011048
// Return true if "name" corresponds to at least one slot definition. This is
11031-
// used to avoid calling update_slot() if is_dunder_name() is true but it's
11032-
// not actually a slot.
11049+
// a more accurate but more expensive test compared to is_dunder_name().
1103311050
static bool
1103411051
has_slotdef(PyObject *name)
1103511052
{
@@ -11288,9 +11305,9 @@ update_all_slots(PyTypeObject* type)
1128811305

1128911306
ASSERT_TYPE_LOCK_HELD();
1129011307

11308+
#ifdef TYPE_SLOT_UPDATE_NEEDS_STOP
1129111309
// Similar to update_slot_world_stopped(), this is required to
1129211310
// avoid races. We do it once here rather than once per-slot.
11293-
#ifdef Py_GIL_DISABLED
1129411311
PyInterpreterState *interp = _PyInterpreterState_GET();
1129511312
_PyEval_StopTheWorld(interp);
1129611313
#endif
@@ -11303,7 +11320,7 @@ update_all_slots(PyTypeObject* type)
1130311320
update_slot(type, p->name_strobj);
1130411321
}
1130511322

11306-
#ifdef Py_GIL_DISABLED
11323+
#ifdef TYPE_SLOT_UPDATE_NEEDS_STOP
1130711324
_PyEval_StartTheWorld(interp);
1130811325
#endif
1130911326
}

0 commit comments

Comments
 (0)