https://github.com/python/cpython/commit/baae9cb159e240ee9474ce7c02f88c233390777a
commit: baae9cb159e240ee9474ce7c02f88c233390777a
branch: main
author: Neil Schemenauer <nas-git...@arctrix.com>
committer: nascheme <nas-git...@arctrix.com>
date: 2025-02-25T21:24:20-08:00
summary:
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.
files:
M Objects/typeobject.c
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 458d78e0502723..f667e56afbbf8b 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -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;
}
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com
