Skip to content

Commit baae9cb

Browse files
authored
gh-117657: Use an atomic store to set type flags. (gh-127588)
The `PyType_HasFeature()` function reads the flags with a relaxed atomic load and without holding the type lock. To avoid data races, use atomic stores if `PyType_Ready()` has already been called.
1 parent 0ef4ffe commit baae9cb

File tree

1 file changed

+83
-47
lines changed

1 file changed

+83
-47
lines changed

Objects/typeobject.c

+83-47
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "pycore_moduleobject.h" // _PyModule_GetDef()
1414
#include "pycore_object.h" // _PyType_HasFeature()
1515
#include "pycore_object_alloc.h" // _PyObject_MallocWithType()
16+
#include "pycore_pyatomic_ft_wrappers.h"
1617
#include "pycore_pyerrors.h" // _PyErr_Occurred()
1718
#include "pycore_pystate.h" // _PyThreadState_GET()
1819
#include "pycore_symtable.h" // _Py_Mangle()
@@ -344,6 +345,39 @@ _PyStaticType_GetBuiltins(void)
344345

345346
/* end static builtin helpers */
346347

348+
static void
349+
type_set_flags(PyTypeObject *tp, unsigned long flags)
350+
{
351+
if (tp->tp_flags & Py_TPFLAGS_READY) {
352+
// It's possible the type object has been exposed to other threads
353+
// if it's been marked ready. In that case, the type lock should be
354+
// held when flags are modified.
355+
ASSERT_TYPE_LOCK_HELD();
356+
}
357+
// Since PyType_HasFeature() reads the flags without holding the type
358+
// lock, we need an atomic store here.
359+
FT_ATOMIC_STORE_ULONG_RELAXED(tp->tp_flags, flags);
360+
}
361+
362+
static void
363+
type_set_flags_with_mask(PyTypeObject *tp, unsigned long mask, unsigned long flags)
364+
{
365+
ASSERT_TYPE_LOCK_HELD();
366+
unsigned long new_flags = (tp->tp_flags & ~mask) | flags;
367+
type_set_flags(tp, new_flags);
368+
}
369+
370+
static void
371+
type_add_flags(PyTypeObject *tp, unsigned long flag)
372+
{
373+
type_set_flags(tp, tp->tp_flags | flag);
374+
}
375+
376+
static void
377+
type_clear_flags(PyTypeObject *tp, unsigned long flag)
378+
{
379+
type_set_flags(tp, tp->tp_flags & ~flag);
380+
}
347381

348382
static inline void
349383
start_readying(PyTypeObject *type)
@@ -357,7 +391,7 @@ start_readying(PyTypeObject *type)
357391
return;
358392
}
359393
assert((type->tp_flags & Py_TPFLAGS_READYING) == 0);
360-
type->tp_flags |= Py_TPFLAGS_READYING;
394+
type_add_flags(type, Py_TPFLAGS_READYING);
361395
}
362396

363397
static inline void
@@ -372,7 +406,7 @@ stop_readying(PyTypeObject *type)
372406
return;
373407
}
374408
assert(type->tp_flags & Py_TPFLAGS_READYING);
375-
type->tp_flags &= ~Py_TPFLAGS_READYING;
409+
type_clear_flags(type, Py_TPFLAGS_READYING);
376410
}
377411

378412
static inline int
@@ -1548,11 +1582,14 @@ type_set_abstractmethods(PyObject *tp, PyObject *value, void *Py_UNUSED(closure)
15481582
return -1;
15491583
}
15501584

1551-
PyType_Modified(type);
1585+
BEGIN_TYPE_LOCK();
1586+
type_modified_unlocked(type);
15521587
if (abstract)
1553-
type->tp_flags |= Py_TPFLAGS_IS_ABSTRACT;
1588+
type_add_flags(type, Py_TPFLAGS_IS_ABSTRACT);
15541589
else
1555-
type->tp_flags &= ~Py_TPFLAGS_IS_ABSTRACT;
1590+
type_clear_flags(type, Py_TPFLAGS_IS_ABSTRACT);
1591+
END_TYPE_LOCK();
1592+
15561593
return 0;
15571594
}
15581595

@@ -3335,7 +3372,7 @@ mro_internal_unlocked(PyTypeObject *type, int initial, PyObject **p_old_mro)
33353372

33363373
// XXX Expand this to Py_TPFLAGS_IMMUTABLETYPE?
33373374
if (!(type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)) {
3338-
PyType_Modified(type);
3375+
type_modified_unlocked(type);
33393376
}
33403377
else {
33413378
/* For static builtin types, this is only called during init
@@ -3941,8 +3978,8 @@ type_new_alloc(type_new_ctx *ctx)
39413978
// Initialize tp_flags.
39423979
// All heap types need GC, since we can create a reference cycle by storing
39433980
// an instance on one of its parents.
3944-
type->tp_flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HEAPTYPE |
3945-
Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC);
3981+
type_set_flags(type, Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HEAPTYPE |
3982+
Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC);
39463983

39473984
// Initialize essential fields
39483985
type->tp_as_async = &et->as_async;
@@ -4175,12 +4212,12 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type)
41754212

41764213
if (ctx->add_weak) {
41774214
assert((type->tp_flags & Py_TPFLAGS_MANAGED_WEAKREF) == 0);
4178-
type->tp_flags |= Py_TPFLAGS_MANAGED_WEAKREF;
4215+
type_add_flags(type, Py_TPFLAGS_MANAGED_WEAKREF);
41794216
type->tp_weaklistoffset = MANAGED_WEAKREF_OFFSET;
41804217
}
41814218
if (ctx->add_dict) {
41824219
assert((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0);
4183-
type->tp_flags |= Py_TPFLAGS_MANAGED_DICT;
4220+
type_add_flags(type, Py_TPFLAGS_MANAGED_DICT);
41844221
type->tp_dictoffset = -1;
41854222
}
41864223

@@ -4978,7 +5015,7 @@ PyType_FromMetaclass(
49785015

49795016
type = &res->ht_type;
49805017
/* The flags must be initialized early, before the GC traverses us */
4981-
type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE;
5018+
type_set_flags(type, spec->flags | Py_TPFLAGS_HEAPTYPE);
49825019

49835020
res->ht_module = Py_XNewRef(module);
49845021

@@ -5739,18 +5776,11 @@ _PyType_CacheGetItemForSpecialization(PyHeapTypeObject *ht, PyObject *descriptor
57395776
return can_cache;
57405777
}
57415778

5742-
static void
5743-
set_flags(PyTypeObject *self, unsigned long mask, unsigned long flags)
5744-
{
5745-
ASSERT_TYPE_LOCK_HELD();
5746-
self->tp_flags = (self->tp_flags & ~mask) | flags;
5747-
}
5748-
57495779
void
57505780
_PyType_SetFlags(PyTypeObject *self, unsigned long mask, unsigned long flags)
57515781
{
57525782
BEGIN_TYPE_LOCK();
5753-
set_flags(self, mask, flags);
5783+
type_set_flags_with_mask(self, mask, flags);
57545784
END_TYPE_LOCK();
57555785
}
57565786

@@ -5781,7 +5811,7 @@ set_flags_recursive(PyTypeObject *self, unsigned long mask, unsigned long flags)
57815811
return;
57825812
}
57835813

5784-
set_flags(self, mask, flags);
5814+
type_set_flags_with_mask(self, mask, flags);
57855815

57865816
PyObject *children = _PyType_GetSubclasses(self);
57875817
if (children == NULL) {
@@ -6116,8 +6146,10 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type,
61166146
clear_static_type_objects(interp, type, isbuiltin, final);
61176147

61186148
if (final) {
6119-
type->tp_flags &= ~Py_TPFLAGS_READY;
6120-
_PyType_SetVersion(type, 0);
6149+
BEGIN_TYPE_LOCK();
6150+
type_clear_flags(type, Py_TPFLAGS_READY);
6151+
set_version_unlocked(type, 0);
6152+
END_TYPE_LOCK();
61216153
}
61226154

61236155
_PyStaticType_ClearWeakRefs(interp, type);
@@ -7866,13 +7898,13 @@ inherit_special(PyTypeObject *type, PyTypeObject *base)
78667898
if (!(type->tp_flags & Py_TPFLAGS_HAVE_GC) &&
78677899
(base->tp_flags & Py_TPFLAGS_HAVE_GC) &&
78687900
(!type->tp_traverse && !type->tp_clear)) {
7869-
type->tp_flags |= Py_TPFLAGS_HAVE_GC;
7901+
type_add_flags(type, Py_TPFLAGS_HAVE_GC);
78707902
if (type->tp_traverse == NULL)
78717903
type->tp_traverse = base->tp_traverse;
78727904
if (type->tp_clear == NULL)
78737905
type->tp_clear = base->tp_clear;
78747906
}
7875-
type->tp_flags |= (base->tp_flags & Py_TPFLAGS_PREHEADER);
7907+
type_add_flags(type, base->tp_flags & Py_TPFLAGS_PREHEADER);
78767908

78777909
if (type->tp_basicsize == 0)
78787910
type->tp_basicsize = base->tp_basicsize;
@@ -7890,38 +7922,40 @@ inherit_special(PyTypeObject *type, PyTypeObject *base)
78907922

78917923
/* Setup fast subclass flags */
78927924
PyObject *mro = lookup_tp_mro(base);
7925+
unsigned long flags = 0;
78937926
if (is_subtype_with_mro(mro, base, (PyTypeObject*)PyExc_BaseException)) {
7894-
type->tp_flags |= Py_TPFLAGS_BASE_EXC_SUBCLASS;
7927+
flags |= Py_TPFLAGS_BASE_EXC_SUBCLASS;
78957928
}
78967929
else if (is_subtype_with_mro(mro, base, &PyType_Type)) {
7897-
type->tp_flags |= Py_TPFLAGS_TYPE_SUBCLASS;
7930+
flags |= Py_TPFLAGS_TYPE_SUBCLASS;
78987931
}
78997932
else if (is_subtype_with_mro(mro, base, &PyLong_Type)) {
7900-
type->tp_flags |= Py_TPFLAGS_LONG_SUBCLASS;
7933+
flags |= Py_TPFLAGS_LONG_SUBCLASS;
79017934
}
79027935
else if (is_subtype_with_mro(mro, base, &PyBytes_Type)) {
7903-
type->tp_flags |= Py_TPFLAGS_BYTES_SUBCLASS;
7936+
flags |= Py_TPFLAGS_BYTES_SUBCLASS;
79047937
}
79057938
else if (is_subtype_with_mro(mro, base, &PyUnicode_Type)) {
7906-
type->tp_flags |= Py_TPFLAGS_UNICODE_SUBCLASS;
7939+
flags |= Py_TPFLAGS_UNICODE_SUBCLASS;
79077940
}
79087941
else if (is_subtype_with_mro(mro, base, &PyTuple_Type)) {
7909-
type->tp_flags |= Py_TPFLAGS_TUPLE_SUBCLASS;
7942+
flags |= Py_TPFLAGS_TUPLE_SUBCLASS;
79107943
}
79117944
else if (is_subtype_with_mro(mro, base, &PyList_Type)) {
7912-
type->tp_flags |= Py_TPFLAGS_LIST_SUBCLASS;
7945+
flags |= Py_TPFLAGS_LIST_SUBCLASS;
79137946
}
79147947
else if (is_subtype_with_mro(mro, base, &PyDict_Type)) {
7915-
type->tp_flags |= Py_TPFLAGS_DICT_SUBCLASS;
7948+
flags |= Py_TPFLAGS_DICT_SUBCLASS;
79167949
}
79177950

79187951
/* Setup some inheritable flags */
79197952
if (PyType_HasFeature(base, _Py_TPFLAGS_MATCH_SELF)) {
7920-
type->tp_flags |= _Py_TPFLAGS_MATCH_SELF;
7953+
flags |= _Py_TPFLAGS_MATCH_SELF;
79217954
}
79227955
if (PyType_HasFeature(base, Py_TPFLAGS_ITEMS_AT_END)) {
7923-
type->tp_flags |= Py_TPFLAGS_ITEMS_AT_END;
7956+
flags |= Py_TPFLAGS_ITEMS_AT_END;
79247957
}
7958+
type_add_flags(type, flags);
79257959
}
79267960

79277961
static int
@@ -8069,7 +8103,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
80698103
if (!type->tp_call &&
80708104
_PyType_HasFeature(base, Py_TPFLAGS_HAVE_VECTORCALL))
80718105
{
8072-
type->tp_flags |= Py_TPFLAGS_HAVE_VECTORCALL;
8106+
type_add_flags(type, Py_TPFLAGS_HAVE_VECTORCALL);
80738107
}
80748108
COPYSLOT(tp_call);
80758109
}
@@ -8103,7 +8137,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
81038137
_PyType_HasFeature(type, Py_TPFLAGS_IMMUTABLETYPE) &&
81048138
_PyType_HasFeature(base, Py_TPFLAGS_METHOD_DESCRIPTOR))
81058139
{
8106-
type->tp_flags |= Py_TPFLAGS_METHOD_DESCRIPTOR;
8140+
type_add_flags(type, Py_TPFLAGS_METHOD_DESCRIPTOR);
81078141
}
81088142
COPYSLOT(tp_descr_set);
81098143
COPYSLOT(tp_dictoffset);
@@ -8418,7 +8452,7 @@ type_ready_inherit_as_structs(PyTypeObject *type, PyTypeObject *base)
84188452
static void
84198453
inherit_patma_flags(PyTypeObject *type, PyTypeObject *base) {
84208454
if ((type->tp_flags & COLLECTION_FLAGS) == 0) {
8421-
type->tp_flags |= base->tp_flags & COLLECTION_FLAGS;
8455+
type_add_flags(type, base->tp_flags & COLLECTION_FLAGS);
84228456
}
84238457
}
84248458

@@ -8533,7 +8567,7 @@ type_ready_set_new(PyTypeObject *type, int initial)
85338567
&& base == &PyBaseObject_Type
85348568
&& !(type->tp_flags & Py_TPFLAGS_HEAPTYPE))
85358569
{
8536-
type->tp_flags |= Py_TPFLAGS_DISALLOW_INSTANTIATION;
8570+
type_add_flags(type, Py_TPFLAGS_DISALLOW_INSTANTIATION);
85378571
}
85388572

85398573
if (!(type->tp_flags & Py_TPFLAGS_DISALLOW_INSTANTIATION)) {
@@ -8580,7 +8614,7 @@ type_ready_managed_dict(PyTypeObject *type)
85808614
}
85818615
}
85828616
if (type->tp_itemsize == 0) {
8583-
type->tp_flags |= Py_TPFLAGS_INLINE_VALUES;
8617+
type_add_flags(type, Py_TPFLAGS_INLINE_VALUES);
85848618
}
85858619
return 0;
85868620
}
@@ -8686,7 +8720,7 @@ type_ready(PyTypeObject *type, int initial)
86868720
}
86878721

86888722
/* All done -- set the ready flag */
8689-
type->tp_flags |= Py_TPFLAGS_READY;
8723+
type_add_flags(type, Py_TPFLAGS_READY);
86908724
stop_readying(type);
86918725

86928726
assert(_PyType_CheckConsistency(type));
@@ -8708,7 +8742,7 @@ PyType_Ready(PyTypeObject *type)
87088742

87098743
/* Historically, all static types were immutable. See bpo-43908 */
87108744
if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) {
8711-
type->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE;
8745+
type_add_flags(type, Py_TPFLAGS_IMMUTABLETYPE);
87128746
/* Static types must be immortal */
87138747
_Py_SetImmortalUntracked((PyObject *)type);
87148748
}
@@ -8738,8 +8772,8 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self,
87388772
if ((self->tp_flags & Py_TPFLAGS_READY) == 0) {
87398773
assert(initial);
87408774

8741-
self->tp_flags |= _Py_TPFLAGS_STATIC_BUILTIN;
8742-
self->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE;
8775+
type_add_flags(self, _Py_TPFLAGS_STATIC_BUILTIN);
8776+
type_add_flags(self, Py_TPFLAGS_IMMUTABLETYPE);
87438777

87448778
assert(NEXT_GLOBAL_VERSION_TAG <= _Py_MAX_GLOBAL_TYPE_VERSION_TAG);
87458779
if (self->tp_version_tag == 0) {
@@ -11126,7 +11160,7 @@ update_one_slot(PyTypeObject *type, pytype_slotdef *p)
1112611160
generic = p->function;
1112711161
if (p->function == slot_tp_call) {
1112811162
/* A generic __call__ is incompatible with vectorcall */
11129-
type->tp_flags &= ~Py_TPFLAGS_HAVE_VECTORCALL;
11163+
type_clear_flags(type, Py_TPFLAGS_HAVE_VECTORCALL);
1113011164
}
1113111165
}
1113211166
Py_DECREF(descr);
@@ -11216,7 +11250,7 @@ update_all_slots(PyTypeObject* type)
1121611250
ASSERT_TYPE_LOCK_HELD();
1121711251

1121811252
/* Clear the VALID_VERSION flag of 'type' and all its subclasses. */
11219-
PyType_Modified(type);
11253+
type_modified_unlocked(type);
1122011254

1122111255
for (p = slotdefs; p->name; p++) {
1122211256
/* update_slot returns int but can't actually fail */
@@ -11492,8 +11526,10 @@ PyType_Freeze(PyTypeObject *type)
1149211526
return -1;
1149311527
}
1149411528

11495-
type->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE;
11496-
PyType_Modified(type);
11529+
BEGIN_TYPE_LOCK();
11530+
type_add_flags(type, Py_TPFLAGS_IMMUTABLETYPE);
11531+
type_modified_unlocked(type);
11532+
END_TYPE_LOCK();
1149711533

1149811534
return 0;
1149911535
}

0 commit comments

Comments
 (0)