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.
This commit is contained in:
Neil Schemenauer 2025-02-25 21:24:20 -08:00 committed by GitHub
parent 0ef4ffeefd
commit baae9cb159
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -13,6 +13,7 @@
#include "pycore_moduleobject.h" // _PyModule_GetDef()
#include "pycore_object.h" // _PyType_HasFeature()
#include "pycore_object_alloc.h" // _PyObject_MallocWithType()
#include "pycore_pyatomic_ft_wrappers.h"
#include "pycore_pyerrors.h" // _PyErr_Occurred()
#include "pycore_pystate.h" // _PyThreadState_GET()
#include "pycore_symtable.h" // _Py_Mangle()
@ -344,6 +345,39 @@ _PyStaticType_GetBuiltins(void)
/* end static builtin helpers */
static void
type_set_flags(PyTypeObject *tp, unsigned long flags)
{
if (tp->tp_flags & Py_TPFLAGS_READY) {
// It's possible the type object has been exposed to other threads
// if it's been marked ready. In that case, the type lock should be
// held when flags are modified.
ASSERT_TYPE_LOCK_HELD();
}
// Since PyType_HasFeature() reads the flags without holding the type
// lock, we need an atomic store here.
FT_ATOMIC_STORE_ULONG_RELAXED(tp->tp_flags, flags);
}
static void
type_set_flags_with_mask(PyTypeObject *tp, unsigned long mask, unsigned long flags)
{
ASSERT_TYPE_LOCK_HELD();
unsigned long new_flags = (tp->tp_flags & ~mask) | flags;
type_set_flags(tp, new_flags);
}
static void
type_add_flags(PyTypeObject *tp, unsigned long flag)
{
type_set_flags(tp, tp->tp_flags | flag);
}
static void
type_clear_flags(PyTypeObject *tp, unsigned long flag)
{
type_set_flags(tp, tp->tp_flags & ~flag);
}
static inline void
start_readying(PyTypeObject *type)
@ -357,7 +391,7 @@ start_readying(PyTypeObject *type)
return;
}
assert((type->tp_flags & Py_TPFLAGS_READYING) == 0);
type->tp_flags |= Py_TPFLAGS_READYING;
type_add_flags(type, Py_TPFLAGS_READYING);
}
static inline void
@ -372,7 +406,7 @@ stop_readying(PyTypeObject *type)
return;
}
assert(type->tp_flags & Py_TPFLAGS_READYING);
type->tp_flags &= ~Py_TPFLAGS_READYING;
type_clear_flags(type, Py_TPFLAGS_READYING);
}
static inline int
@ -1548,11 +1582,14 @@ type_set_abstractmethods(PyObject *tp, PyObject *value, void *Py_UNUSED(closure)
return -1;
}
PyType_Modified(type);
BEGIN_TYPE_LOCK();
type_modified_unlocked(type);
if (abstract)
type->tp_flags |= Py_TPFLAGS_IS_ABSTRACT;
type_add_flags(type, Py_TPFLAGS_IS_ABSTRACT);
else
type->tp_flags &= ~Py_TPFLAGS_IS_ABSTRACT;
type_clear_flags(type, Py_TPFLAGS_IS_ABSTRACT);
END_TYPE_LOCK();
return 0;
}
@ -3335,7 +3372,7 @@ mro_internal_unlocked(PyTypeObject *type, int initial, PyObject **p_old_mro)
// XXX Expand this to Py_TPFLAGS_IMMUTABLETYPE?
if (!(type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)) {
PyType_Modified(type);
type_modified_unlocked(type);
}
else {
/* For static builtin types, this is only called during init
@ -3941,8 +3978,8 @@ type_new_alloc(type_new_ctx *ctx)
// Initialize tp_flags.
// All heap types need GC, since we can create a reference cycle by storing
// an instance on one of its parents.
type->tp_flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HEAPTYPE |
Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC);
type_set_flags(type, Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HEAPTYPE |
Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC);
// Initialize essential fields
type->tp_as_async = &et->as_async;
@ -4175,12 +4212,12 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type)
if (ctx->add_weak) {
assert((type->tp_flags & Py_TPFLAGS_MANAGED_WEAKREF) == 0);
type->tp_flags |= Py_TPFLAGS_MANAGED_WEAKREF;
type_add_flags(type, Py_TPFLAGS_MANAGED_WEAKREF);
type->tp_weaklistoffset = MANAGED_WEAKREF_OFFSET;
}
if (ctx->add_dict) {
assert((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0);
type->tp_flags |= Py_TPFLAGS_MANAGED_DICT;
type_add_flags(type, Py_TPFLAGS_MANAGED_DICT);
type->tp_dictoffset = -1;
}
@ -4978,7 +5015,7 @@ PyType_FromMetaclass(
type = &res->ht_type;
/* The flags must be initialized early, before the GC traverses us */
type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE;
type_set_flags(type, spec->flags | Py_TPFLAGS_HEAPTYPE);
res->ht_module = Py_XNewRef(module);
@ -5739,18 +5776,11 @@ _PyType_CacheGetItemForSpecialization(PyHeapTypeObject *ht, PyObject *descriptor
return can_cache;
}
static void
set_flags(PyTypeObject *self, unsigned long mask, unsigned long flags)
{
ASSERT_TYPE_LOCK_HELD();
self->tp_flags = (self->tp_flags & ~mask) | flags;
}
void
_PyType_SetFlags(PyTypeObject *self, unsigned long mask, unsigned long flags)
{
BEGIN_TYPE_LOCK();
set_flags(self, mask, flags);
type_set_flags_with_mask(self, mask, flags);
END_TYPE_LOCK();
}
@ -5781,7 +5811,7 @@ set_flags_recursive(PyTypeObject *self, unsigned long mask, unsigned long flags)
return;
}
set_flags(self, mask, flags);
type_set_flags_with_mask(self, mask, flags);
PyObject *children = _PyType_GetSubclasses(self);
if (children == NULL) {
@ -6116,8 +6146,10 @@ fini_static_type(PyInterpreterState *interp, PyTypeObject *type,
clear_static_type_objects(interp, type, isbuiltin, final);
if (final) {
type->tp_flags &= ~Py_TPFLAGS_READY;
_PyType_SetVersion(type, 0);
BEGIN_TYPE_LOCK();
type_clear_flags(type, Py_TPFLAGS_READY);
set_version_unlocked(type, 0);
END_TYPE_LOCK();
}
_PyStaticType_ClearWeakRefs(interp, type);
@ -7866,13 +7898,13 @@ inherit_special(PyTypeObject *type, PyTypeObject *base)
if (!(type->tp_flags & Py_TPFLAGS_HAVE_GC) &&
(base->tp_flags & Py_TPFLAGS_HAVE_GC) &&
(!type->tp_traverse && !type->tp_clear)) {
type->tp_flags |= Py_TPFLAGS_HAVE_GC;
type_add_flags(type, Py_TPFLAGS_HAVE_GC);
if (type->tp_traverse == NULL)
type->tp_traverse = base->tp_traverse;
if (type->tp_clear == NULL)
type->tp_clear = base->tp_clear;
}
type->tp_flags |= (base->tp_flags & Py_TPFLAGS_PREHEADER);
type_add_flags(type, base->tp_flags & Py_TPFLAGS_PREHEADER);
if (type->tp_basicsize == 0)
type->tp_basicsize = base->tp_basicsize;
@ -7890,38 +7922,40 @@ inherit_special(PyTypeObject *type, PyTypeObject *base)
/* Setup fast subclass flags */
PyObject *mro = lookup_tp_mro(base);
unsigned long flags = 0;
if (is_subtype_with_mro(mro, base, (PyTypeObject*)PyExc_BaseException)) {
type->tp_flags |= Py_TPFLAGS_BASE_EXC_SUBCLASS;
flags |= Py_TPFLAGS_BASE_EXC_SUBCLASS;
}
else if (is_subtype_with_mro(mro, base, &PyType_Type)) {
type->tp_flags |= Py_TPFLAGS_TYPE_SUBCLASS;
flags |= Py_TPFLAGS_TYPE_SUBCLASS;
}
else if (is_subtype_with_mro(mro, base, &PyLong_Type)) {
type->tp_flags |= Py_TPFLAGS_LONG_SUBCLASS;
flags |= Py_TPFLAGS_LONG_SUBCLASS;
}
else if (is_subtype_with_mro(mro, base, &PyBytes_Type)) {
type->tp_flags |= Py_TPFLAGS_BYTES_SUBCLASS;
flags |= Py_TPFLAGS_BYTES_SUBCLASS;
}
else if (is_subtype_with_mro(mro, base, &PyUnicode_Type)) {
type->tp_flags |= Py_TPFLAGS_UNICODE_SUBCLASS;
flags |= Py_TPFLAGS_UNICODE_SUBCLASS;
}
else if (is_subtype_with_mro(mro, base, &PyTuple_Type)) {
type->tp_flags |= Py_TPFLAGS_TUPLE_SUBCLASS;
flags |= Py_TPFLAGS_TUPLE_SUBCLASS;
}
else if (is_subtype_with_mro(mro, base, &PyList_Type)) {
type->tp_flags |= Py_TPFLAGS_LIST_SUBCLASS;
flags |= Py_TPFLAGS_LIST_SUBCLASS;
}
else if (is_subtype_with_mro(mro, base, &PyDict_Type)) {
type->tp_flags |= Py_TPFLAGS_DICT_SUBCLASS;
flags |= Py_TPFLAGS_DICT_SUBCLASS;
}
/* Setup some inheritable flags */
if (PyType_HasFeature(base, _Py_TPFLAGS_MATCH_SELF)) {
type->tp_flags |= _Py_TPFLAGS_MATCH_SELF;
flags |= _Py_TPFLAGS_MATCH_SELF;
}
if (PyType_HasFeature(base, Py_TPFLAGS_ITEMS_AT_END)) {
type->tp_flags |= Py_TPFLAGS_ITEMS_AT_END;
flags |= Py_TPFLAGS_ITEMS_AT_END;
}
type_add_flags(type, flags);
}
static int
@ -8069,7 +8103,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
if (!type->tp_call &&
_PyType_HasFeature(base, Py_TPFLAGS_HAVE_VECTORCALL))
{
type->tp_flags |= Py_TPFLAGS_HAVE_VECTORCALL;
type_add_flags(type, Py_TPFLAGS_HAVE_VECTORCALL);
}
COPYSLOT(tp_call);
}
@ -8103,7 +8137,7 @@ inherit_slots(PyTypeObject *type, PyTypeObject *base)
_PyType_HasFeature(type, Py_TPFLAGS_IMMUTABLETYPE) &&
_PyType_HasFeature(base, Py_TPFLAGS_METHOD_DESCRIPTOR))
{
type->tp_flags |= Py_TPFLAGS_METHOD_DESCRIPTOR;
type_add_flags(type, Py_TPFLAGS_METHOD_DESCRIPTOR);
}
COPYSLOT(tp_descr_set);
COPYSLOT(tp_dictoffset);
@ -8418,7 +8452,7 @@ type_ready_inherit_as_structs(PyTypeObject *type, PyTypeObject *base)
static void
inherit_patma_flags(PyTypeObject *type, PyTypeObject *base) {
if ((type->tp_flags & COLLECTION_FLAGS) == 0) {
type->tp_flags |= base->tp_flags & COLLECTION_FLAGS;
type_add_flags(type, base->tp_flags & COLLECTION_FLAGS);
}
}
@ -8533,7 +8567,7 @@ type_ready_set_new(PyTypeObject *type, int initial)
&& base == &PyBaseObject_Type
&& !(type->tp_flags & Py_TPFLAGS_HEAPTYPE))
{
type->tp_flags |= Py_TPFLAGS_DISALLOW_INSTANTIATION;
type_add_flags(type, Py_TPFLAGS_DISALLOW_INSTANTIATION);
}
if (!(type->tp_flags & Py_TPFLAGS_DISALLOW_INSTANTIATION)) {
@ -8580,7 +8614,7 @@ type_ready_managed_dict(PyTypeObject *type)
}
}
if (type->tp_itemsize == 0) {
type->tp_flags |= Py_TPFLAGS_INLINE_VALUES;
type_add_flags(type, Py_TPFLAGS_INLINE_VALUES);
}
return 0;
}
@ -8686,7 +8720,7 @@ type_ready(PyTypeObject *type, int initial)
}
/* All done -- set the ready flag */
type->tp_flags |= Py_TPFLAGS_READY;
type_add_flags(type, Py_TPFLAGS_READY);
stop_readying(type);
assert(_PyType_CheckConsistency(type));
@ -8708,7 +8742,7 @@ PyType_Ready(PyTypeObject *type)
/* Historically, all static types were immutable. See bpo-43908 */
if (!(type->tp_flags & Py_TPFLAGS_HEAPTYPE)) {
type->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE;
type_add_flags(type, Py_TPFLAGS_IMMUTABLETYPE);
/* Static types must be immortal */
_Py_SetImmortalUntracked((PyObject *)type);
}
@ -8738,8 +8772,8 @@ init_static_type(PyInterpreterState *interp, PyTypeObject *self,
if ((self->tp_flags & Py_TPFLAGS_READY) == 0) {
assert(initial);
self->tp_flags |= _Py_TPFLAGS_STATIC_BUILTIN;
self->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE;
type_add_flags(self, _Py_TPFLAGS_STATIC_BUILTIN);
type_add_flags(self, Py_TPFLAGS_IMMUTABLETYPE);
assert(NEXT_GLOBAL_VERSION_TAG <= _Py_MAX_GLOBAL_TYPE_VERSION_TAG);
if (self->tp_version_tag == 0) {
@ -11126,7 +11160,7 @@ update_one_slot(PyTypeObject *type, pytype_slotdef *p)
generic = p->function;
if (p->function == slot_tp_call) {
/* A generic __call__ is incompatible with vectorcall */
type->tp_flags &= ~Py_TPFLAGS_HAVE_VECTORCALL;
type_clear_flags(type, Py_TPFLAGS_HAVE_VECTORCALL);
}
}
Py_DECREF(descr);
@ -11216,7 +11250,7 @@ update_all_slots(PyTypeObject* type)
ASSERT_TYPE_LOCK_HELD();
/* Clear the VALID_VERSION flag of 'type' and all its subclasses. */
PyType_Modified(type);
type_modified_unlocked(type);
for (p = slotdefs; p->name; p++) {
/* update_slot returns int but can't actually fail */
@ -11492,8 +11526,10 @@ PyType_Freeze(PyTypeObject *type)
return -1;
}
type->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE;
PyType_Modified(type);
BEGIN_TYPE_LOCK();
type_add_flags(type, Py_TPFLAGS_IMMUTABLETYPE);
type_modified_unlocked(type);
END_TYPE_LOCK();
return 0;
}