From 74d498504d7da1b5b49480fe7842af29e384440e Mon Sep 17 00:00:00 2001 From: Dino Viehland Date: Wed, 10 Jan 2024 12:14:55 -0800 Subject: [PATCH] Make bases/mro accesses thread safe --- Include/internal/pycore_typeobject.h | 4 + Modules/_abc.c | 15 +- Objects/typeobject.c | 324 ++++++++++++++++++++++----- 3 files changed, 277 insertions(+), 66 deletions(-) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index 3a96f7bc83971b7..e2afb714ae05c14 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -9,6 +9,7 @@ extern "C" { #endif #include "pycore_moduleobject.h" // PyModuleObject +#include "pycore_lock.h" // PyMutex /* state */ @@ -21,6 +22,9 @@ struct _types_runtime_state { // bpo-42745: next_version_tag remains shared by all interpreters // because of static types. unsigned int next_version_tag; +#ifdef Py_GIL_DISABLED + PyMutex type_mutex; +#endif }; diff --git a/Modules/_abc.c b/Modules/_abc.c index 9473905243d438d..399ecbbd6a21728 100644 --- a/Modules/_abc.c +++ b/Modules/_abc.c @@ -8,7 +8,6 @@ #include "pycore_object.h" // _PyType_GetSubclasses() #include "pycore_runtime.h" // _Py_ID() #include "pycore_setobject.h" // _PySet_NextEntry() -#include "pycore_typeobject.h" // _PyType_GetMRO() #include "pycore_weakref.h" // _PyWeakref_GET_REF() #include "clinic/_abc.c.h" @@ -744,18 +743,12 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self, Py_DECREF(ok); /* 4. Check if it's a direct subclass. */ - PyObject *mro = _PyType_GetMRO((PyTypeObject *)subclass); - assert(PyTuple_Check(mro)); - for (pos = 0; pos < PyTuple_GET_SIZE(mro); pos++) { - PyObject *mro_item = PyTuple_GET_ITEM(mro, pos); - assert(mro_item != NULL); - if ((PyObject *)self == mro_item) { - if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) { - goto end; - } - result = Py_True; + if (PyType_IsSubtype((PyTypeObject *)subclass, (PyTypeObject *)self)) { + if (_add_to_weak_set(&impl->_abc_cache, subclass) < 0) { goto end; } + result = Py_True; + goto end; } /* 5. Check if it's a subclass of a registered class (recursive). */ diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7587e068e1099a1..98959cf23f33d0d 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -51,6 +51,34 @@ class object "PyObject *" "&PyBaseObject_Type" #define NEXT_VERSION_TAG(interp) \ (interp)->types.next_version_tag +#ifdef Py_GIL_DISABLED + +// There's a global lock for mutation of types. This avoids having to take +// additonal locks while doing various subclass processing which may result +// in odd behaviors w.r.t. running with the GIL as the outer type lock could +// be released and reacquired during a subclass update if there's contention +// on the subclass lock. +#define BEGIN_TYPE_LOCK() \ + { \ + _PyCriticalSection _cs; \ + _PyCriticalSection_Begin(&_cs, &_PyRuntime.types.type_mutex); \ + +#define END_TYPE_LOCK() \ + _PyCriticalSection_End(&_cs); \ + } + +#define ASSERT_TYPE_LOCK_HELD() \ + _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&_PyRuntime.types.type_mutex) + +#else + +#define BEGIN_TYPE_LOCK() +#define END_TYPE_LOCK() +#define ASSERT_TYPE_LOCK_HELD() + +#endif + + typedef struct PySlot_Offset { short subslot_offset; short slot_offset; @@ -278,8 +306,14 @@ lookup_tp_bases(PyTypeObject *self) PyObject * _PyType_GetBases(PyTypeObject *self) { - /* It returns a borrowed reference. */ - return lookup_tp_bases(self); + PyObject *res; + + BEGIN_TYPE_LOCK(); + res = lookup_tp_bases(self); + Py_INCREF(res); + END_TYPE_LOCK() + + return res; } static inline void @@ -329,14 +363,19 @@ clear_tp_bases(PyTypeObject *self) static inline PyObject * lookup_tp_mro(PyTypeObject *self) { + ASSERT_TYPE_LOCK_HELD(); return self->tp_mro; } PyObject * _PyType_GetMRO(PyTypeObject *self) { - /* It returns a borrowed reference. */ - return lookup_tp_mro(self); + PyObject *mro; + BEGIN_TYPE_LOCK(); + mro = lookup_tp_mro(self); + Py_INCREF(mro); + END_TYPE_LOCK() + return mro; } static inline void @@ -759,8 +798,10 @@ PyType_Watch(int watcher_id, PyObject* obj) return -1; } // ensure we will get a callback on the next modification + BEGIN_TYPE_LOCK() assign_version_tag(interp, type); type->tp_watched |= (1 << watcher_id); + END_TYPE_LOCK() return 0; } @@ -780,8 +821,8 @@ PyType_Unwatch(int watcher_id, PyObject* obj) return 0; } -void -PyType_Modified(PyTypeObject *type) +static void +type_modified_unlocked(PyTypeObject *type) { /* Invalidate any cached data for the specified type and all subclasses. This function is called after the base @@ -847,6 +888,22 @@ PyType_Modified(PyTypeObject *type) } } +void +PyType_Modified(PyTypeObject *type) +{ + // Quick check without the lock held + if (!_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)) { + return; + } + + BEGIN_TYPE_LOCK() + type_modified_unlocked(type); + END_TYPE_LOCK() +} + +static int +is_subtype_unlocked(PyTypeObject *a, PyTypeObject *b); + static void type_mro_modified(PyTypeObject *type, PyObject *bases) { /* @@ -865,6 +922,7 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) { int custom = !Py_IS_TYPE(type, &PyType_Type); int unbound; + ASSERT_TYPE_LOCK_HELD(); if (custom) { PyObject *mro_meth, *type_mro_meth; mro_meth = lookup_maybe_method( @@ -890,7 +948,7 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) { PyObject *b = PyTuple_GET_ITEM(bases, i); PyTypeObject *cls = _PyType_CAST(b); - if (!PyType_IsSubtype(type, cls)) { + if (!is_subtype_unlocked(type, cls)) { goto clear; } } @@ -910,6 +968,8 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) { static int assign_version_tag(PyInterpreterState *interp, PyTypeObject *type) { + ASSERT_TYPE_LOCK_HELD(); + /* Ensure that the tp_version_tag is valid and set Py_TPFLAGS_VALID_VERSION_TAG. To respect the invariant, this must first be done on all super classes. Return 0 if this @@ -955,7 +1015,11 @@ assign_version_tag(PyInterpreterState *interp, PyTypeObject *type) int PyUnstable_Type_AssignVersionTag(PyTypeObject *type) { PyInterpreterState *interp = _PyInterpreterState_GET(); - return assign_version_tag(interp, type); + int assigned; + BEGIN_TYPE_LOCK() + assigned = assign_version_tag(interp, type); + END_TYPE_LOCK() + return assigned; } @@ -1177,21 +1241,28 @@ type_set_abstractmethods(PyTypeObject *type, PyObject *value, void *context) static PyObject * type_get_bases(PyTypeObject *type, void *context) { - PyObject *bases = lookup_tp_bases(type); + PyObject *bases = _PyType_GetBases(type); if (bases == NULL) { Py_RETURN_NONE; } - return Py_NewRef(bases); + return bases; } static PyObject * type_get_mro(PyTypeObject *type, void *context) { - PyObject *mro = lookup_tp_mro(type); + PyObject *mro; + + BEGIN_TYPE_LOCK() + mro = lookup_tp_mro(type); if (mro == NULL) { - Py_RETURN_NONE; + mro = Py_None; + } else { + Py_INCREF(mro); } - return Py_NewRef(mro); + + END_TYPE_LOCK() + return mro; } static PyTypeObject *best_base(PyObject *); @@ -1213,6 +1284,8 @@ static int recurse_down_subclasses(PyTypeObject *type, PyObject *name, static int mro_hierarchy(PyTypeObject *type, PyObject *temp) { + ASSERT_TYPE_LOCK_HELD(); + PyObject *old_mro; int res = mro_internal(type, &old_mro); if (res <= 0) { @@ -1276,7 +1349,7 @@ mro_hierarchy(PyTypeObject *type, PyObject *temp) } static int -type_set_bases(PyTypeObject *type, PyObject *new_bases, void *context) +type_set_bases_unlocked(PyTypeObject *type, PyObject *new_bases, void *context) { // Check arguments if (!check_set_special_type_attr(type, new_bases, "__bases__")) { @@ -1307,7 +1380,7 @@ type_set_bases(PyTypeObject *type, PyObject *new_bases, void *context) } PyTypeObject *base = (PyTypeObject*)ob; - if (PyType_IsSubtype(base, type) || + if (is_subtype_unlocked(base, type) || /* In case of reentering here again through a custom mro() the above check is not enough since it relies on base->tp_mro which would gonna be updated inside @@ -1391,7 +1464,6 @@ type_set_bases(PyTypeObject *type, PyObject *new_bases, void *context) } } Py_DECREF(temp); - bail: if (lookup_tp_bases(type) == new_bases) { assert(type->tp_base == new_base); @@ -1411,6 +1483,16 @@ type_set_bases(PyTypeObject *type, PyObject *new_bases, void *context) return -1; } +static int +type_set_bases(PyTypeObject *type, PyObject *new_bases, void *context) +{ + int res; + BEGIN_TYPE_LOCK(); + res = type_set_bases_unlocked(type, new_bases, context); + END_TYPE_LOCK(); + return res; +} + static PyObject * type_dict(PyTypeObject *type, void *context) { @@ -2149,11 +2231,12 @@ type_is_subtype_base_chain(PyTypeObject *a, PyTypeObject *b) return (b == &PyBaseObject_Type); } -int -PyType_IsSubtype(PyTypeObject *a, PyTypeObject *b) +static int +is_subtype_unlocked(PyTypeObject *a, PyTypeObject *b) { PyObject *mro; + ASSERT_TYPE_LOCK_HELD(); mro = lookup_tp_mro(a); if (mro != NULL) { /* Deal with multiple inheritance without recursion @@ -2172,6 +2255,16 @@ PyType_IsSubtype(PyTypeObject *a, PyTypeObject *b) return type_is_subtype_base_chain(a, b); } +int +PyType_IsSubtype(PyTypeObject *a, PyTypeObject *b) +{ + int res; + BEGIN_TYPE_LOCK(); + res = is_subtype_unlocked(a, b); + END_TYPE_LOCK() + return res; +} + /* Routines to do a method lookup in the type without looking in the instance dictionary (so we can't use PyObject_GetAttr) but still binding it to the instance. @@ -2531,8 +2624,10 @@ pmerge(PyObject *acc, PyObject **to_merge, Py_ssize_t to_merge_size) } static PyObject * -mro_implementation(PyTypeObject *type) +mro_implementation_unlocked(PyTypeObject *type) { + ASSERT_TYPE_LOCK_HELD(); + if (!_PyType_IsReady(type)) { if (PyType_Ready(type) < 0) return NULL; @@ -2612,6 +2707,16 @@ mro_implementation(PyTypeObject *type) return result; } +static PyObject * +mro_implementation(PyTypeObject *type) +{ + PyObject *mro; + BEGIN_TYPE_LOCK() + mro = mro_implementation_unlocked(type); + END_TYPE_LOCK() + return mro; +} + /*[clinic input] type.mro @@ -2650,7 +2755,7 @@ mro_check(PyTypeObject *type, PyObject *mro) } PyTypeObject *base = (PyTypeObject*)obj; - if (!PyType_IsSubtype(solid, solid_base(base))) { + if (!is_subtype_unlocked(solid, solid_base(base))) { PyErr_Format( PyExc_TypeError, "mro() returned base with unsuitable layout ('%.500s')", @@ -2681,6 +2786,9 @@ mro_invoke(PyTypeObject *type) { PyObject *mro_result; PyObject *new_mro; + + ASSERT_TYPE_LOCK_HELD(); + const int custom = !Py_IS_TYPE(type, &PyType_Type); if (custom) { @@ -2693,7 +2801,7 @@ mro_invoke(PyTypeObject *type) Py_DECREF(mro_meth); } else { - mro_result = mro_implementation(type); + mro_result = mro_implementation_unlocked(type); } if (mro_result == NULL) return NULL; @@ -2740,8 +2848,10 @@ mro_invoke(PyTypeObject *type) - Returns -1 in case of an error. */ static int -mro_internal(PyTypeObject *type, PyObject **p_old_mro) +mro_internal_unlocked(PyTypeObject *type, PyObject **p_old_mro) { + ASSERT_TYPE_LOCK_HELD(); + PyObject *new_mro, *old_mro; int reent; @@ -2786,6 +2896,16 @@ mro_internal(PyTypeObject *type, PyObject **p_old_mro) return 1; } +static int +mro_internal(PyTypeObject *type, PyObject **p_old_mro) +{ + int res; + BEGIN_TYPE_LOCK() + res = mro_internal_unlocked(type, p_old_mro); + END_TYPE_LOCK() + return res; +} + /* Calculate the best base amongst multiple base classes. This is the first one that's on the path to the "solid base". */ @@ -4613,6 +4733,9 @@ PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) { assert(PyType_Check(type)); + PyObject *res = NULL; + BEGIN_TYPE_LOCK() + PyObject *mro = lookup_tp_mro(type); // The type must be ready assert(mro != NULL); @@ -4632,15 +4755,19 @@ PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) PyHeapTypeObject *ht = (PyHeapTypeObject*)super; PyObject *module = ht->ht_module; if (module && _PyModule_GetDef(module) == def) { - return module; + res = module; + break; } } + END_TYPE_LOCK() - PyErr_Format( - PyExc_TypeError, - "PyType_GetModuleByDef: No superclass of '%s' has the given module", - type->tp_name); - return NULL; + if (res == NULL) { + PyErr_Format( + PyExc_TypeError, + "PyType_GetModuleByDef: No superclass of '%s' has the given module", + type->tp_name); + } + return res; } void * @@ -4678,6 +4805,8 @@ PyObject_GetItemData(PyObject *obj) static PyObject * find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) { + ASSERT_TYPE_LOCK_HELD(); + Py_hash_t hash; if (!PyUnicode_CheckExact(name) || (hash = _PyASCIIObject_CAST(name)->hash) == -1) @@ -4849,7 +4978,7 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) assert(_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)); OBJECT_STAT_INC_COND(type_cache_hits, !is_dunder_name(name)); OBJECT_STAT_INC_COND(type_cache_dunder_hits, is_dunder_name(name)); - return entry->value; + return entry->value; } #endif OBJECT_STAT_INC_COND(type_cache_misses, !is_dunder_name(name)); @@ -4858,7 +4987,19 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) /* We may end up clearing live exceptions below, so make sure it's ours. */ assert(!PyErr_Occurred()); + // We need to atomically do the lookup and capture the version before + // anyone else can modify our mro or mutate the type. + + int has_version = 0; + int version = 0; + BEGIN_TYPE_LOCK() res = find_name_in_mro(type, name, &error); + if (MCACHE_CACHEABLE_NAME(name)) { + has_version = assign_version_tag(interp, type); + version = type->tp_version_tag; + } + END_TYPE_LOCK() + /* Only put NULL results into cache if there was no error. */ if (error) { /* It's not ideal to clear the error condition, @@ -4875,11 +5016,11 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) return NULL; } - if (MCACHE_CACHEABLE_NAME(name) && assign_version_tag(interp, type)) { + if (has_version) { #if Py_GIL_DISABLED - update_cache_gil_disabled(entry, name, type->tp_version_tag, res); + update_cache_gil_disabled(entry, name, version, res); #else - update_cache(entry, name, type->tp_version_tag, res); + update_cache(entry, name, version, res); #endif assert(_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)); } @@ -5044,6 +5185,8 @@ type_setattro(PyObject *self, PyObject *name, PyObject *value) /* Will fail in _PyObject_GenericSetAttrWithDict. */ Py_INCREF(name); } + + BEGIN_TYPE_LOCK() res = _PyObject_GenericSetAttrWithDict((PyObject *)type, name, value, NULL); if (res == 0) { /* Clear the VALID_VERSION flag of 'type' and all its @@ -5051,13 +5194,15 @@ type_setattro(PyObject *self, PyObject *name, PyObject *value) update_subclasses() recursion in update_slot(), but carefully: they each have their own conditions on which to stop recursing into subclasses. */ - PyType_Modified(type); + type_modified_unlocked(type); if (is_dunder_name(name)) { res = update_slot(type, name); } assert(_PyType_CheckConsistency(type)); } + END_TYPE_LOCK() + Py_DECREF(name); return res; } @@ -6860,28 +7005,28 @@ inherit_special(PyTypeObject *type, PyTypeObject *base) #undef COPYVAL /* Setup fast subclass flags */ - if (PyType_IsSubtype(base, (PyTypeObject*)PyExc_BaseException)) { + if (is_subtype_unlocked(base, (PyTypeObject*)PyExc_BaseException)) { type->tp_flags |= Py_TPFLAGS_BASE_EXC_SUBCLASS; } - else if (PyType_IsSubtype(base, &PyType_Type)) { + else if (is_subtype_unlocked(base, &PyType_Type)) { type->tp_flags |= Py_TPFLAGS_TYPE_SUBCLASS; } - else if (PyType_IsSubtype(base, &PyLong_Type)) { + else if (is_subtype_unlocked(base, &PyLong_Type)) { type->tp_flags |= Py_TPFLAGS_LONG_SUBCLASS; } - else if (PyType_IsSubtype(base, &PyBytes_Type)) { + else if (is_subtype_unlocked(base, &PyBytes_Type)) { type->tp_flags |= Py_TPFLAGS_BYTES_SUBCLASS; } - else if (PyType_IsSubtype(base, &PyUnicode_Type)) { + else if (is_subtype_unlocked(base, &PyUnicode_Type)) { type->tp_flags |= Py_TPFLAGS_UNICODE_SUBCLASS; } - else if (PyType_IsSubtype(base, &PyTuple_Type)) { + else if (is_subtype_unlocked(base, &PyTuple_Type)) { type->tp_flags |= Py_TPFLAGS_TUPLE_SUBCLASS; } - else if (PyType_IsSubtype(base, &PyList_Type)) { + else if (is_subtype_unlocked(base, &PyList_Type)) { type->tp_flags |= Py_TPFLAGS_LIST_SUBCLASS; } - else if (PyType_IsSubtype(base, &PyDict_Type)) { + else if (is_subtype_unlocked(base, &PyDict_Type)) { type->tp_flags |= Py_TPFLAGS_DICT_SUBCLASS; } @@ -7320,6 +7465,8 @@ type_ready_preheader(PyTypeObject *type) static int type_ready_mro(PyTypeObject *type) { + ASSERT_TYPE_LOCK_HELD(); + if (type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) { if (!_Py_IsMainInterpreter(_PyInterpreterState_GET())) { assert(lookup_tp_mro(type) != NULL); @@ -7329,7 +7476,7 @@ type_ready_mro(PyTypeObject *type) } /* Calculate method resolution order */ - if (mro_internal(type, NULL) < 0) { + if (mro_internal_unlocked(type, NULL) < 0) { return -1; } PyObject *mro = lookup_tp_mro(type); @@ -7393,6 +7540,8 @@ inherit_patma_flags(PyTypeObject *type, PyTypeObject *base) { static int type_ready_inherit(PyTypeObject *type) { + ASSERT_TYPE_LOCK_HELD(); + /* Inherit special flags from dominant base */ PyTypeObject *base = type->tp_base; if (base != NULL) { @@ -7585,6 +7734,8 @@ type_ready_post_checks(PyTypeObject *type) static int type_ready(PyTypeObject *type, int rerunbuiltin) { + ASSERT_TYPE_LOCK_HELD(); + _PyObject_ASSERT((PyObject *)type, !is_readying(type)); start_readying(type); @@ -7672,7 +7823,16 @@ PyType_Ready(PyTypeObject *type) type->tp_flags |= Py_TPFLAGS_IMMUTABLETYPE; } - return type_ready(type, 0); + int res; + BEGIN_TYPE_LOCK() + if (!(type->tp_flags & Py_TPFLAGS_READY)) { + res = type_ready(type, 0); + } else { + res = 0; + assert(_PyType_CheckConsistency(type)); + } + END_TYPE_LOCK() + return res; } int @@ -7702,7 +7862,10 @@ _PyStaticType_InitBuiltin(PyInterpreterState *interp, PyTypeObject *self) static_builtin_state_init(interp, self); - int res = type_ready(self, !ismain); + int res; + BEGIN_TYPE_LOCK(); + res = type_ready(self, !ismain); + END_TYPE_LOCK() if (res < 0) { static_builtin_state_clear(interp, self); } @@ -8104,9 +8267,12 @@ wrap_delitem(PyObject *self, PyObject *args, void *wrapped) https://mail.python.org/pipermail/python-dev/2003-April/034535.html */ static int -hackcheck(PyObject *self, setattrofunc func, const char *what) +hackcheck_unlocked(PyObject *self, setattrofunc func, const char *what) { PyTypeObject *type = Py_TYPE(self); + + ASSERT_TYPE_LOCK_HELD(); + PyObject *mro = lookup_tp_mro(type); if (!mro) { /* Probably ok not to check the call in this case. */ @@ -8148,6 +8314,20 @@ hackcheck(PyObject *self, setattrofunc func, const char *what) return 1; } +static int +hackcheck(PyObject *self, setattrofunc func, const char *what) +{ + if (!PyType_Check(self)) { + return 1; + } + + int res; + BEGIN_TYPE_LOCK(); + res = hackcheck_unlocked(self, func, what); + END_TYPE_LOCK() + return res; +} + static PyObject * wrap_setattr(PyObject *self, PyObject *args, void *wrapped) { @@ -9317,7 +9497,7 @@ slot_bf_getbuffer(PyObject *self, Py_buffer *buffer, int flags) } static int -releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer) +releasebuffer_maybe_call_super_unlocked(PyObject *self, Py_buffer *buffer, releasebufferproc *base_releasebuffer) { PyTypeObject *self_type = Py_TYPE(self); PyObject *mro = lookup_tp_mro(self_type); @@ -9338,7 +9518,7 @@ releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer) if (i >= n) return -1; - releasebufferproc base_releasebuffer = NULL; + *base_releasebuffer = NULL; for (; i < n; i++) { PyObject *obj = PyTuple_GET_ITEM(mro, i); if (!PyType_Check(obj)) { @@ -9348,15 +9528,28 @@ releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer) if (base_type->tp_as_buffer != NULL && base_type->tp_as_buffer->bf_releasebuffer != NULL && base_type->tp_as_buffer->bf_releasebuffer != slot_bf_releasebuffer) { - base_releasebuffer = base_type->tp_as_buffer->bf_releasebuffer; + *base_releasebuffer = base_type->tp_as_buffer->bf_releasebuffer; break; } } - if (base_releasebuffer != NULL) { + return 0; +} + +static int +releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer) +{ + int res; + releasebufferproc base_releasebuffer; + + BEGIN_TYPE_LOCK(); + res = releasebuffer_maybe_call_super_unlocked(self, buffer, &base_releasebuffer); + END_TYPE_LOCK(); + + if (res == 0) { base_releasebuffer(self, buffer); } - return 0; + return res; } static void @@ -9892,6 +10085,8 @@ resolve_slotdups(PyTypeObject *type, PyObject *name) static pytype_slotdef * update_one_slot(PyTypeObject *type, pytype_slotdef *p) { + ASSERT_TYPE_LOCK_HELD(); + PyObject *descr; PyWrapperDescrObject *d; @@ -9940,7 +10135,7 @@ update_one_slot(PyTypeObject *type, pytype_slotdef *p) d = (PyWrapperDescrObject *)descr; if ((specific == NULL || specific == d->d_wrapped) && d->d_base->wrapper == p->wrapper && - PyType_IsSubtype(type, PyDescr_TYPE(d))) + is_subtype_unlocked(type, PyDescr_TYPE(d))) { specific = d->d_wrapped; } @@ -10005,6 +10200,8 @@ update_one_slot(PyTypeObject *type, pytype_slotdef *p) static int update_slots_callback(PyTypeObject *type, void *data) { + ASSERT_TYPE_LOCK_HELD(); + pytype_slotdef **pp = (pytype_slotdef **)data; for (; *pp; pp++) { update_one_slot(type, *pp); @@ -10021,6 +10218,7 @@ update_slot(PyTypeObject *type, PyObject *name) pytype_slotdef **pp; int offset; + ASSERT_TYPE_LOCK_HELD(); assert(PyUnicode_CheckExact(name)); assert(PyUnicode_CHECK_INTERNED(name)); @@ -10054,10 +10252,17 @@ update_slot(PyTypeObject *type, PyObject *name) static void fixup_slot_dispatchers(PyTypeObject *type) { + // This lock isn't strictly necessary because the type has not been + // exposed to anyone else yet, but update_ont_slot calls find_name_in_mro + // where we'd like to assert that the tyep is locked. + BEGIN_TYPE_LOCK() + assert(!PyErr_Occurred()); for (pytype_slotdef *p = slotdefs; p->name; ) { p = update_one_slot(type, p); } + + END_TYPE_LOCK() } static void @@ -10065,6 +10270,8 @@ update_all_slots(PyTypeObject* type) { pytype_slotdef *p; + ASSERT_TYPE_LOCK_HELD(); + /* Clear the VALID_VERSION flag of 'type' and all its subclasses. */ PyType_Modified(type); @@ -10337,7 +10544,15 @@ _super_lookup_descr(PyTypeObject *su_type, PyTypeObject *su_obj_type, PyObject * PyObject *mro, *res; Py_ssize_t i, n; + BEGIN_TYPE_LOCK(); mro = lookup_tp_mro(su_obj_type); + /* keep a strong reference to mro because su_obj_type->tp_mro can be + replaced during PyDict_GetItemRef(dict, name, &res) and because + another thread can modify it after we end the critical section + below */ + Py_XINCREF(mro); + END_TYPE_LOCK() + if (mro == NULL) return NULL; @@ -10350,12 +10565,11 @@ _super_lookup_descr(PyTypeObject *su_type, PyTypeObject *su_obj_type, PyObject * break; } i++; /* skip su->type (if any) */ - if (i >= n) + if (i >= n) { + Py_DECREF(mro); return NULL; + } - /* keep a strong reference to mro because su_obj_type->tp_mro can be - replaced during PyDict_GetItemRef(dict, name, &res) */ - Py_INCREF(mro); do { PyObject *obj = PyTuple_GET_ITEM(mro, i); PyObject *dict = lookup_tp_dict(_PyType_CAST(obj));