Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-117657: Use an atomic store to set type flags. #127588

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 83 additions & 47 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -343,6 +344,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) {
nascheme marked this conversation as resolved.
Show resolved Hide resolved
// 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);
nascheme marked this conversation as resolved.
Show resolved Hide resolved
}

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)
Expand All @@ -356,7 +390,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
Expand All @@ -371,7 +405,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
Expand Down Expand Up @@ -1528,11 +1562,14 @@ type_set_abstractmethods(PyTypeObject *type, PyObject *value, void *context)
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;
}

Expand Down Expand Up @@ -3300,7 +3337,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
Expand Down Expand Up @@ -3906,8 +3943,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;
Expand Down Expand Up @@ -4140,12 +4177,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;
}

Expand Down Expand Up @@ -4943,7 +4980,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);

Expand Down Expand Up @@ -5679,18 +5716,11 @@ _PyType_CacheInitForSpecialization(PyHeapTypeObject *type, PyObject *init,
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();
}

Expand Down Expand Up @@ -5721,7 +5751,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) {
Expand Down Expand Up @@ -6055,8 +6085,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);
Expand Down Expand Up @@ -7804,13 +7836,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;
Expand All @@ -7828,38 +7860,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
Expand Down Expand Up @@ -8007,7 +8041,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);
}
Expand Down Expand Up @@ -8041,7 +8075,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);
Expand Down Expand Up @@ -8356,7 +8390,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);
}
}

Expand Down Expand Up @@ -8471,7 +8505,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)) {
Expand Down Expand Up @@ -8518,7 +8552,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;
}
Expand Down Expand Up @@ -8624,7 +8658,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));
Expand All @@ -8646,7 +8680,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);
}
Expand Down Expand Up @@ -8676,8 +8710,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) {
Expand Down Expand Up @@ -11060,7 +11094,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);
Expand Down Expand Up @@ -11150,7 +11184,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 */
Expand Down Expand Up @@ -11426,8 +11460,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;
}
Expand Down
Loading