Skip to content

Commit 92b4143

Browse files
naschememiss-islington
authored andcommitted
gh-151377: Fix races updating type slots and subclasses (GH-155370)
(cherry picked from commit 07624ef) Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>
1 parent 042df0a commit 92b4143

2 files changed

Lines changed: 100 additions & 38 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix races in free-threaded builds when updating type slots for newly created
2+
classes and when removing entries from a base type's subclasses dictionary.

Objects/typeobject.c

Lines changed: 98 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -772,17 +772,14 @@ _PyType_HasSubclasses(PyTypeObject *self)
772772
return 1;
773773
}
774774

775-
PyObject*
776-
_PyType_GetSubclasses(PyTypeObject *self)
775+
static int
776+
get_subclasses_unlocked(PyTypeObject *self, PyObject *list)
777777
{
778-
PyObject *list = PyList_New(0);
779-
if (list == NULL) {
780-
return NULL;
781-
}
778+
ASSERT_TYPE_LOCK_HELD();
782779

783780
PyObject *subclasses = lookup_tp_subclasses(self); // borrowed ref
784781
if (subclasses == NULL) {
785-
return list;
782+
return 0;
786783
}
787784
assert(PyDict_CheckExact(subclasses));
788785
// The loop cannot modify tp_subclasses, there is no need
@@ -796,12 +793,33 @@ _PyType_GetSubclasses(PyTypeObject *self)
796793
continue;
797794
}
798795

799-
if (PyList_Append(list, _PyObject_CAST(subclass)) < 0) {
800-
Py_DECREF(list);
801-
Py_DECREF(subclass);
802-
return NULL;
803-
}
796+
int res = PyList_Append(list, _PyObject_CAST(subclass));
804797
Py_DECREF(subclass);
798+
if (res < 0) {
799+
return -1;
800+
}
801+
}
802+
return 0;
803+
}
804+
805+
PyObject*
806+
_PyType_GetSubclasses(PyTypeObject *self)
807+
{
808+
PyObject *list = PyList_New(0);
809+
if (list == NULL) {
810+
return NULL;
811+
}
812+
813+
// The type lock protects tp_subclasses from being mutated while we
814+
// iterate over it (e.g. by add_subclass() or by remove_subclass() when a
815+
// subclass is deallocated).
816+
int res;
817+
BEGIN_TYPE_LOCK();
818+
res = get_subclasses_unlocked(self, list);
819+
END_TYPE_LOCK();
820+
821+
if (res < 0) {
822+
Py_CLEAR(list);
805823
}
806824
return list;
807825
}
@@ -3944,6 +3962,8 @@ static PyObject *object_new(PyTypeObject *, PyObject *, PyObject *);
39443962
static int object_init(PyObject *, PyObject *, PyObject *);
39453963
static int update_slot(PyTypeObject *, PyObject *, slot_update_t *update);
39463964
static void fixup_slot_dispatchers(PyTypeObject *);
3965+
static int type_ready(PyTypeObject *, int, int);
3966+
static int type_ready_publish(PyTypeObject *, int);
39473967
static int type_new_set_names(PyTypeObject *);
39483968
static int type_new_init_subclass(PyTypeObject *, PyObject *);
39493969
static bool has_slotdef(PyObject *);
@@ -4950,13 +4970,10 @@ type_new_impl(type_new_ctx *ctx)
49504970
}
49514971

49524972
/* Initialize the rest */
4953-
if (PyType_Ready(type) < 0) {
4973+
if (type_ready_publish(type, 1) < 0) {
49544974
goto error;
49554975
}
49564976

4957-
// Put the proper slots in place
4958-
fixup_slot_dispatchers(type);
4959-
49604977
if (!_PyDict_HasOnlyStringKeys(type->tp_dict)) {
49614978
if (PyErr_WarnFormat(
49624979
PyExc_RuntimeWarning,
@@ -4977,10 +4994,6 @@ type_new_impl(type_new_ctx *ctx)
49774994
}
49784995

49794996
assert(_PyType_CheckConsistency(type));
4980-
#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG) && SIZEOF_VOID_P > 4
4981-
// After this point, other threads can potentally use this type.
4982-
((PyObject*)type)->ob_flags |= _Py_TYPE_REVEALED_FLAG;
4983-
#endif
49844997

49854998
return (PyObject *)type;
49864999

@@ -5721,8 +5734,7 @@ type_from_slots_or_spec(
57215734
* After this call we should generally only touch up what's
57225735
* accessible to Python code, like __dict__.
57235736
*/
5724-
5725-
if (PyType_Ready(type) < 0) {
5737+
if (type_ready_publish(type, 0) < 0) {
57265738
goto finally;
57275739
}
57285740

@@ -5782,10 +5794,6 @@ type_from_slots_or_spec(
57825794
}
57835795

57845796
assert(_PyType_CheckConsistency(type));
5785-
#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG) && SIZEOF_VOID_P > 4
5786-
// After this point, other threads can potentally use this type.
5787-
((PyObject*)type)->ob_flags |= _Py_TYPE_REVEALED_FLAG;
5788-
#endif
57895797

57905798
finally:
57915799
if (PyErr_Occurred()) {
@@ -6851,7 +6859,9 @@ type_dealloc_common(PyTypeObject *type)
68516859
PyObject *bases = lookup_tp_bases(type);
68526860
if (bases != NULL) {
68536861
PyObject *exc = PyErr_GetRaisedException();
6862+
BEGIN_TYPE_LOCK();
68546863
remove_all_subclasses(type, bases);
6864+
END_TYPE_LOCK();
68556865
PyErr_SetRaisedException(exc);
68566866
}
68576867
}
@@ -9525,7 +9535,7 @@ type_ready_post_checks(PyTypeObject *type)
95259535

95269536

95279537
static int
9528-
type_ready(PyTypeObject *type, int initial)
9538+
type_ready(PyTypeObject *type, int initial, int add_subclasses)
95299539
{
95309540
ASSERT_TYPE_LOCK_HELD();
95319541

@@ -9578,8 +9588,10 @@ type_ready(PyTypeObject *type, int initial)
95789588
if (type_ready_set_hash(type) < 0) {
95799589
goto error;
95809590
}
9581-
if (type_ready_add_subclasses(type) < 0) {
9582-
goto error;
9591+
if (add_subclasses) {
9592+
if (type_ready_add_subclasses(type) < 0) {
9593+
goto error;
9594+
}
95839595
}
95849596
if (initial) {
95859597
if (type_ready_managed_dict(type) < 0) {
@@ -9590,11 +9602,13 @@ type_ready(PyTypeObject *type, int initial)
95909602
}
95919603
}
95929604

9593-
/* All done -- set the ready flag */
9594-
if (initial) {
9595-
type_add_flags(type, Py_TPFLAGS_READY);
9596-
} else {
9597-
assert(type->tp_flags & Py_TPFLAGS_READY);
9605+
if (add_subclasses) {
9606+
/* All done -- set the ready flag */
9607+
if (initial) {
9608+
type_add_flags(type, Py_TPFLAGS_READY);
9609+
} else {
9610+
assert(type->tp_flags & Py_TPFLAGS_READY);
9611+
}
95989612
}
95999613

96009614
stop_readying(type);
@@ -9607,6 +9621,46 @@ type_ready(PyTypeObject *type, int initial)
96079621
return -1;
96089622
}
96099623

9624+
static int
9625+
type_ready_publish(PyTypeObject *type, int fix_slots)
9626+
{
9627+
int res;
9628+
BEGIN_TYPE_LOCK();
9629+
res = type_ready(type, 1, 0);
9630+
if (res == 0) {
9631+
assert(!(type->tp_flags & Py_TPFLAGS_READY));
9632+
assert(!is_readying(type));
9633+
9634+
if (fix_slots) {
9635+
// Put the proper slots in place and only then publish the type as
9636+
// a subclass of its bases. Since the type is not reachable by
9637+
// other threads before it is published, the slots can be updated
9638+
// without stopping the world. This step is skipped for
9639+
// type_from_slots_or_spec().
9640+
fixup_slot_dispatchers(type);
9641+
}
9642+
9643+
// Set the ready flag before revealing the type since type_add_flags()
9644+
// may only be used on types that are not yet revealed.
9645+
type_add_flags(type, Py_TPFLAGS_READY);
9646+
9647+
#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG) && SIZEOF_VOID_P > 4
9648+
// Mark the type as revealed while still holding the type lock.
9649+
// Threads can only find the type through the subclasses of its bases,
9650+
// which is done below with the lock held. So, they cannot see the
9651+
// type before the flag is set.
9652+
((PyObject*)type)->ob_flags |= _Py_TYPE_REVEALED_FLAG;
9653+
#endif
9654+
9655+
res = type_ready_add_subclasses(type);
9656+
if (res == 0) {
9657+
assert(_PyType_CheckConsistency(type));
9658+
}
9659+
}
9660+
END_TYPE_LOCK();
9661+
return res;
9662+
}
9663+
96109664
int
96119665
PyType_Ready(PyTypeObject *type)
96129666
{
@@ -9626,7 +9680,7 @@ PyType_Ready(PyTypeObject *type)
96269680
int res;
96279681
BEGIN_TYPE_LOCK();
96289682
if (!(type->tp_flags & Py_TPFLAGS_READY)) {
9629-
res = type_ready(type, 1);
9683+
res = type_ready(type, 1, 1);
96309684
} else {
96319685
res = 0;
96329686
assert(_PyType_CheckConsistency(type));
@@ -9667,7 +9721,7 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self,
96679721

96689722
int res;
96699723
BEGIN_TYPE_LOCK();
9670-
res = type_ready(self, initial);
9724+
res = type_ready(self, initial, 1);
96719725
END_TYPE_LOCK();
96729726
if (res < 0) {
96739727
_PyStaticType_ClearWeakRefs(interp, self);
@@ -12126,13 +12180,19 @@ update_slot(PyTypeObject *type, PyObject *name, slot_update_t *queued_updates)
1212612180

1212712181
/* Store the proper functions in the slot dispatches at class (type)
1212812182
definition time, based upon which operations the class overrides in its
12129-
dict. */
12183+
dict. The type must not be revealed to other threads yet, so that the
12184+
slots can be updated directly rather than with the world stopped. */
1213012185
static void
1213112186
fixup_slot_dispatchers(PyTypeObject *type)
1213212187
{
12188+
ASSERT_TYPE_LOCK_HELD();
12189+
ASSERT_WORLD_STOPPED_OR_NEW_TYPE(type);
1213312190
assert(!PyErr_Occurred());
1213412191
for (pytype_slotdef *p = slotdefs; p->name; ) {
12135-
update_one_slot(type, p, &p, NULL);
12192+
int rv = update_one_slot(type, p, &p, NULL);
12193+
// always returns 0 if queued_updates == NULL
12194+
assert (rv == 0);
12195+
(void)rv;
1213612196
}
1213712197
}
1213812198

0 commit comments

Comments
 (0)