Skip to content

Commit 549beea

Browse files
naschemelpyu001
andcommitted
gh-151377: Fix races updating type slots and subclasses
* Avoid data race in fixup_slot_dispatchers(). * Set _Py_TYPE_REVEALED_FLAG before the type is published * PyType_FromMetaclass() now uses type_ready()/type_ready_publish() rather than PyType_Ready(), like type_new_impl() does, so that it publishes the type the same way. The flag macros are only defined for debug builds now, since that is the only build where the flag exists. * Hold the type lock while iterating tp_subclasses. * Hold the type lock while the slots of a new type are set up. fixup_slot_dispatchers() previously ran without the type lock held. Another thread could assign to a special method of a base between the point where we look up the special methods in the bases and the point where the type is added to the subclasses of its bases. * Do the ready, the slot fixup and the publishing of the type in one type lock critical section. A concurrent assignment to a base now either happens before we look up the special methods, or it finds the type in the subclasses of its bases. The slots are still stored directly rather than with the world stopped, since the type is not reachable by other threads yet. Co-authored-by: lipengyu <lipengyu@kylinos.cn> Assisted-by: Claude Opus 5
1 parent 115400b commit 549beea

2 files changed

Lines changed: 136 additions & 46 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: 134 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,23 @@ types_world_is_stopped(void)
8181
#endif
8282

8383
// Checks that the type has not yet been revealed (exposed) to other
84-
// threads. The _Py_TYPE_REVEALED_FLAG flag is set by type_new() and
85-
// PyType_FromMetaclass() to indicate that a newly initialized type might be
86-
// revealed. We only have ob_flags on 64-bit platforms.
87-
#if SIZEOF_VOID_P > 4
88-
#define TYPE_IS_REVEALED(tp) ((((PyObject *)(tp))->ob_flags & _Py_TYPE_REVEALED_FLAG) != 0)
84+
// threads. The _Py_TYPE_REVEALED_FLAG flag is set by type_ready_publish(),
85+
// just before the type is added to the subclasses of its bases. That is the
86+
// point where other threads can find it. The flag only exists on 64-bit
87+
// platforms (we only have ob_flags there) and in debug builds.
88+
//
89+
// The flag is set with the type lock held and it is never cleared. A thread
90+
// that can see the type has either found it through the subclasses of its
91+
// bases, which requires the type lock, or the world is stopped. So, plain
92+
// loads and stores are enough here.
93+
#if defined(Py_DEBUG) && SIZEOF_VOID_P > 4
94+
#define TYPE_IS_REVEALED(tp) \
95+
((((PyObject *)(tp))->ob_flags & _Py_TYPE_REVEALED_FLAG) != 0)
96+
#define TYPE_SET_REVEALED(tp) \
97+
((void)(((PyObject *)(tp))->ob_flags |= _Py_TYPE_REVEALED_FLAG))
8998
#else
9099
#define TYPE_IS_REVEALED(tp) 0
100+
#define TYPE_SET_REVEALED(tp) ((void)0)
91101
#endif
92102

93103
#ifdef Py_DEBUG
@@ -173,6 +183,7 @@ type_lock_allow_release(void)
173183
#define END_TYPE_DICT_LOCK()
174184
#define ASSERT_TYPE_LOCK_HELD()
175185
#define TYPE_IS_REVEALED(tp) 0
186+
#define TYPE_SET_REVEALED(tp) ((void)0)
176187
#define ASSERT_WORLD_STOPPED_OR_NEW_TYPE(tp)
177188
#define ASSERT_NEW_TYPE_OR_LOCKED(tp)
178189
#define types_world_is_stopped() 1
@@ -758,17 +769,14 @@ _PyType_HasSubclasses(PyTypeObject *self)
758769
return 1;
759770
}
760771

761-
PyObject*
762-
_PyType_GetSubclasses(PyTypeObject *self)
772+
static int
773+
get_subclasses_unlocked(PyTypeObject *self, PyObject *list)
763774
{
764-
PyObject *list = PyList_New(0);
765-
if (list == NULL) {
766-
return NULL;
767-
}
775+
ASSERT_TYPE_LOCK_HELD();
768776

769777
PyObject *subclasses = lookup_tp_subclasses(self); // borrowed ref
770778
if (subclasses == NULL) {
771-
return list;
779+
return 0;
772780
}
773781
assert(PyDict_CheckExact(subclasses));
774782
// The loop cannot modify tp_subclasses, there is no need
@@ -782,12 +790,34 @@ _PyType_GetSubclasses(PyTypeObject *self)
782790
continue;
783791
}
784792

785-
if (PyList_Append(list, _PyObject_CAST(subclass)) < 0) {
786-
Py_DECREF(list);
787-
Py_DECREF(subclass);
788-
return NULL;
789-
}
793+
int res = PyList_Append(list, _PyObject_CAST(subclass));
790794
Py_DECREF(subclass);
795+
if (res < 0) {
796+
return -1;
797+
}
798+
}
799+
return 0;
800+
}
801+
802+
PyObject*
803+
_PyType_GetSubclasses(PyTypeObject *self)
804+
{
805+
PyObject *list = PyList_New(0);
806+
if (list == NULL) {
807+
return NULL;
808+
}
809+
810+
// The type lock protects tp_subclasses from being mutated while we
811+
// iterate over it (e.g. by add_subclass() or by remove_subclass() when a
812+
// subclass is deallocated). Note that the lock is not actually acquired
813+
// if it is already held by this thread or if the world is stopped.
814+
int res;
815+
BEGIN_TYPE_LOCK();
816+
res = get_subclasses_unlocked(self, list);
817+
END_TYPE_LOCK();
818+
819+
if (res < 0) {
820+
Py_CLEAR(list);
791821
}
792822
return list;
793823
}
@@ -3873,7 +3903,9 @@ static void object_dealloc(PyObject *);
38733903
static PyObject *object_new(PyTypeObject *, PyObject *, PyObject *);
38743904
static int object_init(PyObject *, PyObject *, PyObject *);
38753905
static int update_slot(PyTypeObject *, PyObject *, slot_update_t *update);
3876-
static void fixup_slot_dispatchers(PyTypeObject *);
3906+
static int fixup_slot_dispatchers(PyTypeObject *);
3907+
static int type_ready(PyTypeObject *, int, int);
3908+
static int type_ready_publish(PyTypeObject *, int);
38773909
static int type_new_set_names(PyTypeObject *);
38783910
static int type_new_init_subclass(PyTypeObject *, PyObject *);
38793911
static bool has_slotdef(PyObject *);
@@ -4879,14 +4911,30 @@ type_new_impl(type_new_ctx *ctx)
48794911
goto error;
48804912
}
48814913

4882-
/* Initialize the rest */
4883-
if (PyType_Ready(type) < 0) {
4914+
/* Initialize the rest, put the proper slots in place and only then
4915+
publish the type as a subclass of its bases. Since the type is not
4916+
reachable by other threads before it is published, the slots can be
4917+
updated without stopping the world.
4918+
4919+
All of it is done with the type lock held. Otherwise a thread that
4920+
assigns to a special method of a base could look for the subclasses to
4921+
update after we have looked up the special methods in the bases but
4922+
before the type is published, and the type would be left with a stale
4923+
slot. */
4924+
int res;
4925+
BEGIN_TYPE_LOCK();
4926+
res = type_ready(type, 1, 0);
4927+
if (res == 0) {
4928+
res = fixup_slot_dispatchers(type);
4929+
}
4930+
if (res == 0) {
4931+
res = type_ready_publish(type, 1);
4932+
}
4933+
END_TYPE_LOCK();
4934+
if (res < 0) {
48844935
goto error;
48854936
}
48864937

4887-
// Put the proper slots in place
4888-
fixup_slot_dispatchers(type);
4889-
48904938
if (!_PyDict_HasOnlyStringKeys(type->tp_dict)) {
48914939
if (PyErr_WarnFormat(
48924940
PyExc_RuntimeWarning,
@@ -4907,10 +4955,6 @@ type_new_impl(type_new_ctx *ctx)
49074955
}
49084956

49094957
assert(_PyType_CheckConsistency(type));
4910-
#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG) && SIZEOF_VOID_P > 4
4911-
// After this point, other threads can potentally use this type.
4912-
((PyObject*)type)->ob_flags |= _Py_TYPE_REVEALED_FLAG;
4913-
#endif
49144958

49154959
return (PyObject *)type;
49164960

@@ -5652,7 +5696,15 @@ type_from_slots_or_spec(
56525696
* accessible to Python code, like __dict__.
56535697
*/
56545698

5655-
if (PyType_Ready(type) < 0) {
5699+
BEGIN_TYPE_LOCK();
5700+
r = type_ready(type, 1, 0);
5701+
if (r == 0) {
5702+
/* The type is only revealed to other threads once it is fully
5703+
initialized. */
5704+
r = type_ready_publish(type, 1);
5705+
}
5706+
END_TYPE_LOCK();
5707+
if (r < 0) {
56565708
goto finally;
56575709
}
56585710

@@ -5712,10 +5764,6 @@ type_from_slots_or_spec(
57125764
}
57135765

57145766
assert(_PyType_CheckConsistency(type));
5715-
#if defined(Py_GIL_DISABLED) && defined(Py_DEBUG) && SIZEOF_VOID_P > 4
5716-
// After this point, other threads can potentally use this type.
5717-
((PyObject*)type)->ob_flags |= _Py_TYPE_REVEALED_FLAG;
5718-
#endif
57195767

57205768
finally:
57215769
if (PyErr_Occurred()) {
@@ -6692,7 +6740,9 @@ type_dealloc_common(PyTypeObject *type)
66926740
PyObject *bases = lookup_tp_bases(type);
66936741
if (bases != NULL) {
66946742
PyObject *exc = PyErr_GetRaisedException();
6743+
BEGIN_TYPE_LOCK();
66956744
remove_all_subclasses(type, bases);
6745+
END_TYPE_LOCK();
66966746
PyErr_SetRaisedException(exc);
66976747
}
66986748
}
@@ -9368,7 +9418,7 @@ type_ready_post_checks(PyTypeObject *type)
93689418

93699419

93709420
static int
9371-
type_ready(PyTypeObject *type, int initial)
9421+
type_ready(PyTypeObject *type, int initial, int add_subclasses)
93729422
{
93739423
ASSERT_TYPE_LOCK_HELD();
93749424

@@ -9423,8 +9473,10 @@ type_ready(PyTypeObject *type, int initial)
94239473
if (type_ready_set_hash(type) < 0) {
94249474
goto error;
94259475
}
9426-
if (type_ready_add_subclasses(type) < 0) {
9427-
goto error;
9476+
if (add_subclasses) {
9477+
if (type_ready_add_subclasses(type) < 0) {
9478+
goto error;
9479+
}
94289480
}
94299481
if (initial) {
94309482
if (type_ready_managed_dict(type) < 0) {
@@ -9435,23 +9487,53 @@ type_ready(PyTypeObject *type, int initial)
94359487
}
94369488
}
94379489

9438-
/* All done -- set the ready flag */
9439-
if (initial) {
9440-
type_add_flags(type, Py_TPFLAGS_READY);
9441-
} else {
9442-
assert(type->tp_flags & Py_TPFLAGS_READY);
9490+
if (add_subclasses) {
9491+
/* All done -- set the ready flag */
9492+
if (initial) {
9493+
type_add_flags(type, Py_TPFLAGS_READY);
9494+
} else {
9495+
assert(type->tp_flags & Py_TPFLAGS_READY);
9496+
}
94439497
}
94449498

94459499
stop_readying(type);
94469500

9447-
assert(_PyType_CheckConsistency(type));
9501+
if (add_subclasses) {
9502+
assert(_PyType_CheckConsistency(type));
9503+
}
94489504
return 0;
94499505

94509506
error:
94519507
stop_readying(type);
94529508
return -1;
94539509
}
94549510

9511+
static int
9512+
type_ready_publish(PyTypeObject *type, int initial)
9513+
{
9514+
ASSERT_TYPE_LOCK_HELD();
9515+
assert(initial);
9516+
assert(!(type->tp_flags & Py_TPFLAGS_READY));
9517+
assert(!is_readying(type));
9518+
9519+
/* Set the ready flag before revealing the type since type_add_flags()
9520+
may only be used on types that are not yet revealed. */
9521+
type_add_flags(type, Py_TPFLAGS_READY);
9522+
9523+
/* Mark the type as revealed while still holding the type lock. Threads
9524+
can only find the type through the subclasses of its bases, which is
9525+
done below with the lock held. So, they cannot see the type before
9526+
the flag is set. */
9527+
TYPE_SET_REVEALED(type);
9528+
9529+
if (type_ready_add_subclasses(type) < 0) {
9530+
return -1;
9531+
}
9532+
9533+
assert(_PyType_CheckConsistency(type));
9534+
return 0;
9535+
}
9536+
94559537
int
94569538
PyType_Ready(PyTypeObject *type)
94579539
{
@@ -9471,7 +9553,7 @@ PyType_Ready(PyTypeObject *type)
94719553
int res;
94729554
BEGIN_TYPE_LOCK();
94739555
if (!(type->tp_flags & Py_TPFLAGS_READY)) {
9474-
res = type_ready(type, 1);
9556+
res = type_ready(type, 1, 1);
94759557
} else {
94769558
res = 0;
94779559
assert(_PyType_CheckConsistency(type));
@@ -9512,7 +9594,7 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self,
95129594

95139595
int res;
95149596
BEGIN_TYPE_LOCK();
9515-
res = type_ready(self, initial);
9597+
res = type_ready(self, initial, 1);
95169598
END_TYPE_LOCK();
95179599
if (res < 0) {
95189600
_PyStaticType_ClearWeakRefs(interp, self);
@@ -11972,14 +12054,20 @@ update_slot(PyTypeObject *type, PyObject *name, slot_update_t *queued_updates)
1197212054

1197312055
/* Store the proper functions in the slot dispatches at class (type)
1197412056
definition time, based upon which operations the class overrides in its
11975-
dict. */
11976-
static void
12057+
dict. The type must not be revealed to other threads yet, so that the
12058+
slots can be updated directly rather than with the world stopped. */
12059+
static int
1197712060
fixup_slot_dispatchers(PyTypeObject *type)
1197812061
{
12062+
ASSERT_TYPE_LOCK_HELD();
12063+
ASSERT_WORLD_STOPPED_OR_NEW_TYPE(type);
1197912064
assert(!PyErr_Occurred());
1198012065
for (pytype_slotdef *p = slotdefs; p->name; ) {
11981-
update_one_slot(type, p, &p, NULL);
12066+
if (update_one_slot(type, p, &p, NULL) < 0) {
12067+
return -1;
12068+
}
1198212069
}
12070+
return 0;
1198312071
}
1198412072

1198512073
#ifdef Py_GIL_DISABLED

0 commit comments

Comments
 (0)