diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h index 5314a70436b..e10d4828536 100644 --- a/Include/cpython/pyatomic.h +++ b/Include/cpython/pyatomic.h @@ -469,6 +469,9 @@ _Py_atomic_store_int_release(int *obj, int value); static inline int _Py_atomic_load_int_acquire(const int *obj); +static inline uint32_t +_Py_atomic_load_uint32_acquire(const uint32_t *obj); + // --- _Py_atomic_fence ------------------------------------------------------ diff --git a/Include/cpython/pyatomic_gcc.h b/Include/cpython/pyatomic_gcc.h index 70f2b7e1b57..4095e1873d8 100644 --- a/Include/cpython/pyatomic_gcc.h +++ b/Include/cpython/pyatomic_gcc.h @@ -495,6 +495,9 @@ static inline int _Py_atomic_load_int_acquire(const int *obj) { return __atomic_load_n(obj, __ATOMIC_ACQUIRE); } +static inline uint32_t +_Py_atomic_load_uint32_acquire(const uint32_t *obj) +{ return __atomic_load_n(obj, __ATOMIC_ACQUIRE); } // --- _Py_atomic_fence ------------------------------------------------------ diff --git a/Include/cpython/pyatomic_msc.h b/Include/cpython/pyatomic_msc.h index 601a0cf65af..b5c1ec94112 100644 --- a/Include/cpython/pyatomic_msc.h +++ b/Include/cpython/pyatomic_msc.h @@ -938,6 +938,17 @@ _Py_atomic_load_int_acquire(const int *obj) #endif } +static inline uint32_t +_Py_atomic_load_uint32_acquire(const uint32_t *obj) +{ +#if defined(_M_X64) || defined(_M_IX86) + return *(uint32_t volatile *)obj; +#elif defined(_M_ARM64) + return (int)__ldar32((uint32_t volatile *)obj); +#else +# error "no implementation of _Py_atomic_load_uint32_acquire" +#endif +} // --- _Py_atomic_fence ------------------------------------------------------ diff --git a/Include/cpython/pyatomic_std.h b/Include/cpython/pyatomic_std.h index a05bfaec47e..6c934a2c5e7 100644 --- a/Include/cpython/pyatomic_std.h +++ b/Include/cpython/pyatomic_std.h @@ -870,6 +870,13 @@ _Py_atomic_load_int_acquire(const int *obj) memory_order_acquire); } +static inline uint32_t +_Py_atomic_load_uint32_acquire(const uint32_t *obj) +{ + _Py_USING_STD; + return atomic_load_explicit((const _Atomic(uint32_t)*)obj, + memory_order_acquire); +} // --- _Py_atomic_fence ------------------------------------------------------ diff --git a/Include/internal/pycore_critical_section.h b/Include/internal/pycore_critical_section.h index 30820a24f5b..9163b5cf0f2 100644 --- a/Include/internal/pycore_critical_section.h +++ b/Include/internal/pycore_critical_section.h @@ -293,7 +293,8 @@ _PyCriticalSection_SuspendAll(PyThreadState *tstate); #ifdef Py_GIL_DISABLED static inline void -_PyCriticalSection_AssertHeld(PyMutex *mutex) { +_PyCriticalSection_AssertHeld(PyMutex *mutex) +{ #ifdef Py_DEBUG PyThreadState *tstate = _PyThreadState_GET(); uintptr_t prev = tstate->critical_section; diff --git a/Include/internal/pycore_lock.h b/Include/internal/pycore_lock.h index 18a8896d97a..674a1d170fe 100644 --- a/Include/internal/pycore_lock.h +++ b/Include/internal/pycore_lock.h @@ -251,6 +251,39 @@ PyAPI_FUNC(void) _PyRWMutex_RUnlock(_PyRWMutex *rwmutex); PyAPI_FUNC(void) _PyRWMutex_Lock(_PyRWMutex *rwmutex); PyAPI_FUNC(void) _PyRWMutex_Unlock(_PyRWMutex *rwmutex); +// Similar to linux seqlock: https://en.wikipedia.org/wiki/Seqlock +// We use a sequence number to lock the writer, an even sequence means we're unlocked, an odd +// sequence means we're locked. Readers will read the sequence before attempting to read the +// underlying data and then read the sequence number again after reading the data. If the +// sequence has not changed the data is valid. +// +// Differs a little bit in that we use CAS on sequence as the lock, instead of a seperate spin lock. +// The writer can also detect that the undelering data has not changed and abandon the write +// and restore the previous sequence. +typedef struct { + uint32_t sequence; +} _PySeqLock; + +// Lock the sequence lock for the writer +PyAPI_FUNC(void) _PySeqLock_LockWrite(_PySeqLock *seqlock); + +// Unlock the sequence lock and move to the next sequence number. +PyAPI_FUNC(void) _PySeqLock_UnlockWrite(_PySeqLock *seqlock); + +// Abandon the current update indicating that no mutations have occured +// and restore the previous sequence value. +PyAPI_FUNC(void) _PySeqLock_AbandonWrite(_PySeqLock *seqlock); + +// Begin a read operation and return the current sequence number. +PyAPI_FUNC(uint32_t) _PySeqLock_BeginRead(_PySeqLock *seqlock); + +// End the read operation and confirm that the sequence number has not changed. +// Returns 1 if the read was successful or 0 if the read should be re-tried. +PyAPI_FUNC(uint32_t) _PySeqLock_EndRead(_PySeqLock *seqlock, uint32_t previous); + +// Check if the lock was held during a fork and clear the lock. Returns 1 +// if the lock was held and any associated datat should be cleared. +PyAPI_FUNC(uint32_t) _PySeqLock_AfterFork(_PySeqLock *seqlock); #ifdef __cplusplus } diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index c03c3d766be..664f6fb212a 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,7 @@ struct _types_runtime_state { // bpo-42745: next_version_tag remains shared by all interpreters // because of static types. unsigned int next_version_tag; + PyMutex type_mutex; }; @@ -28,6 +30,9 @@ struct _types_runtime_state { // see _PyType_Lookup(). struct type_cache_entry { unsigned int version; // initialized from type->tp_version_tag +#ifdef Py_GIL_DISABLED + _PySeqLock sequence; +#endif PyObject *name; // reference to exactly a str or None PyObject *value; // borrowed reference or NULL }; @@ -74,7 +79,7 @@ struct types_state { extern PyStatus _PyTypes_InitTypes(PyInterpreterState *); extern void _PyTypes_FiniTypes(PyInterpreterState *); extern void _PyTypes_Fini(PyInterpreterState *); - +extern void _PyTypes_AfterFork(void); /* other API */ diff --git a/Modules/_abc.c b/Modules/_abc.c index 9473905243d..399ecbbd6a2 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 c65d0ec2aca..e0711dfe854 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -6,6 +6,7 @@ #include "pycore_code.h" // CO_FAST_FREE #include "pycore_dict.h" // _PyDict_KeysSize() #include "pycore_frame.h" // _PyInterpreterFrame +#include "pycore_lock.h" // _PySeqLock_* #include "pycore_long.h" // _PyLong_IsNegative() #include "pycore_memoryobject.h" // _PyMemoryView_FromBufferProc() #include "pycore_modsupport.h" // _PyArg_NoKwnames() @@ -52,6 +53,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; @@ -279,8 +308,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 @@ -330,14 +365,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 @@ -646,9 +686,15 @@ type_cache_clear(struct type_cache *cache, PyObject *value) { for (Py_ssize_t i = 0; i < (1 << MCACHE_SIZE_EXP); i++) { struct type_cache_entry *entry = &cache->hashtable[i]; +#ifdef Py_GIL_DISABLED + _PySeqLock_LockWrite(&entry->sequence); +#endif entry->version = 0; Py_XSETREF(entry->name, _Py_XNewRef(value)); entry->value = NULL; +#ifdef Py_GIL_DISABLED + _PySeqLock_UnlockWrite(&entry->sequence); +#endif } } @@ -760,8 +806,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; } @@ -781,8 +829,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 @@ -848,6 +896,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) { /* @@ -866,6 +930,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( @@ -891,7 +956,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; } } @@ -913,6 +978,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 @@ -961,7 +1028,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; } @@ -1183,21 +1254,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 *); @@ -1219,6 +1297,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) { @@ -1282,7 +1362,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__")) { @@ -1313,7 +1393,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 @@ -1418,6 +1498,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) { @@ -2156,11 +2246,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 @@ -2179,6 +2270,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. @@ -2538,8 +2639,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; @@ -2619,6 +2722,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 @@ -2657,7 +2770,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')", @@ -2688,6 +2801,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) { @@ -2700,7 +2816,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; @@ -2747,8 +2863,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; @@ -2793,6 +2911,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". */ @@ -4631,6 +4759,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); @@ -4650,15 +4781,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 * @@ -4696,6 +4831,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) @@ -4765,6 +4902,62 @@ is_dunder_name(PyObject *name) return 0; } +static void +update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int version_tag, PyObject *value) +{ + entry->version = version_tag; + entry->value = value; /* borrowed */ + assert(_PyASCIIObject_CAST(name)->hash != -1); + OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name); + // We're releasing this under the lock for simplicity sake because it's always a + // exact unicode object or Py_None so it's safe to do so. + Py_SETREF(entry->name, Py_NewRef(name)); +} + +#if Py_GIL_DISABLED + +#define TYPE_CACHE_IS_UPDATING(sequence) (sequence & 0x01) + +static void +update_cache_gil_disabled(struct type_cache_entry *entry, PyObject *name, + unsigned int version_tag, PyObject *value) +{ + _PySeqLock_LockWrite(&entry->sequence); + + // update the entry + if (entry->name == name && + entry->value == value && + entry->version == version_tag) { + // We raced with another update, bail and restore previous sequence. + _PySeqLock_AbandonWrite(&entry->sequence); + return; + } + + update_cache(entry, name, version_tag, value); + + // Then update sequence to the next valid value + _PySeqLock_UnlockWrite(&entry->sequence); +} + +#endif + +void +_PyTypes_AfterFork() +{ +#ifdef Py_GIL_DISABLED + struct type_cache *cache = get_type_cache(); + for (Py_ssize_t i = 0; i < (1 << MCACHE_SIZE_EXP); i++) { + struct type_cache_entry *entry = &cache->hashtable[i]; + if (_PySeqLock_AfterFork(&entry->sequence)) { + // Entry was in the process of updating while forking, clear it... + entry->value = NULL; + Py_SETREF(entry->name, Py_None); + entry->version = 0; + } + } +#endif +} + /* Internal API to look for a name through the MRO. This returns a borrowed reference, and doesn't set an exception! */ PyObject * @@ -4777,6 +4970,27 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) unsigned int h = MCACHE_HASH_METHOD(type, name); struct type_cache *cache = get_type_cache(); struct type_cache_entry *entry = &cache->hashtable[h]; +#ifdef Py_GIL_DISABLED + // synchronize-with other writing threads by doing an acquire load on the sequence + while (1) { + int sequence = _PySeqLock_BeginRead(&entry->sequence); + if (_Py_atomic_load_uint32_relaxed(&entry->version) == type->tp_version_tag && + _Py_atomic_load_ptr_relaxed(&entry->name) == 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)); + PyObject *value = _Py_atomic_load_ptr_relaxed(&entry->value); + + // If the sequence is still valid then we're done + if (_PySeqLock_EndRead(&entry->sequence, sequence)) { + return value; + } + } else { + // cache miss + break; + } + } +#else if (entry->version == type->tp_version_tag && entry->name == name) { assert(_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)); @@ -4784,13 +4998,27 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) OBJECT_STAT_INC_COND(type_cache_dunder_hits, is_dunder_name(name)); return entry->value; } +#endif OBJECT_STAT_INC_COND(type_cache_misses, !is_dunder_name(name)); OBJECT_STAT_INC_COND(type_cache_dunder_misses, is_dunder_name(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; + assert(!has_version || _PyType_HasFeature(type, Py_TPFLAGS_VALID_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, @@ -4807,15 +5035,12 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) return NULL; } - if (MCACHE_CACHEABLE_NAME(name) && assign_version_tag(interp, type)) { - h = MCACHE_HASH_METHOD(type, name); - struct type_cache_entry *entry = &cache->hashtable[h]; - entry->version = type->tp_version_tag; - entry->value = res; /* borrowed */ - assert(_PyASCIIObject_CAST(name)->hash != -1); - OBJECT_STAT_INC_COND(type_cache_collisions, entry->name != Py_None && entry->name != name); - assert(_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)); - Py_SETREF(entry->name, Py_NewRef(name)); + if (has_version) { +#if Py_GIL_DISABLED + update_cache_gil_disabled(entry, name, version, res); +#else + update_cache(entry, name, version, res); +#endif } return res; } @@ -4978,6 +5203,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 @@ -4985,13 +5212,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; } @@ -6796,28 +7025,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; } @@ -7256,6 +7485,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); @@ -7265,7 +7496,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); @@ -7329,6 +7560,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) { @@ -7521,6 +7754,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); @@ -7608,7 +7843,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 @@ -7638,7 +7882,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); } @@ -8040,9 +8287,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. */ @@ -8084,6 +8334,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) { @@ -9252,13 +9516,13 @@ fail: return -1; } -static int -releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer) +static releasebufferproc +releasebuffer_maybe_call_super_unlocked(PyObject *self, Py_buffer *buffer) { PyTypeObject *self_type = Py_TYPE(self); PyObject *mro = lookup_tp_mro(self_type); if (mro == NULL) { - return -1; + return NULL; } assert(PyTuple_Check(mro)); @@ -9272,9 +9536,8 @@ releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer) } i++; /* skip self_type */ if (i >= n) - return -1; + return NULL; - releasebufferproc base_releasebuffer = NULL; for (; i < n; i++) { PyObject *obj = PyTuple_GET_ITEM(mro, i); if (!PyType_Check(obj)) { @@ -9284,15 +9547,25 @@ 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; - break; + return base_type->tp_as_buffer->bf_releasebuffer; } } + return NULL; +} + +static void +releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer) +{ + releasebufferproc base_releasebuffer; + + BEGIN_TYPE_LOCK(); + base_releasebuffer = releasebuffer_maybe_call_super_unlocked(self, buffer); + END_TYPE_LOCK(); + if (base_releasebuffer != NULL) { base_releasebuffer(self, buffer); } - return 0; } static void @@ -9369,11 +9642,7 @@ static void slot_bf_releasebuffer(PyObject *self, Py_buffer *buffer) { releasebuffer_call_python(self, buffer); - if (releasebuffer_maybe_call_super(self, buffer) < 0) { - if (PyErr_Occurred()) { - PyErr_WriteUnraisable(self); - } - } + releasebuffer_maybe_call_super(self, buffer); } static PyObject * @@ -9828,6 +10097,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; @@ -9876,7 +10147,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; } @@ -9941,6 +10212,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); @@ -9957,6 +10230,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)); @@ -9990,10 +10264,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 @@ -10001,6 +10282,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); @@ -10273,7 +10556,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; @@ -10286,12 +10577,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)); diff --git a/Python/lock.c b/Python/lock.c index f0ff1176941..bf0143654bd 100644 --- a/Python/lock.c +++ b/Python/lock.c @@ -459,3 +459,74 @@ _PyRWMutex_Unlock(_PyRWMutex *rwmutex) _PyParkingLot_UnparkAll(&rwmutex->bits); } } + +#define SEQLOCK_IS_UPDATING(sequence) (sequence & 0x01) + +void _PySeqLock_LockWrite(_PySeqLock *seqlock) +{ + // lock the entry by setting by moving to an odd sequence number + uint32_t prev = _Py_atomic_load_uint32_relaxed(&seqlock->sequence); + while (1) { + if (SEQLOCK_IS_UPDATING(prev)) { + // Someone else is currently updating the cache + _Py_yield(); + prev = _Py_atomic_load_uint32_relaxed(&seqlock->sequence); + } + else if (_Py_atomic_compare_exchange_uint32(&seqlock->sequence, &prev, prev + 1)) { + // We've locked the cache + break; + } + else { + _Py_yield(); + } + } +} + +void _PySeqLock_AbandonWrite(_PySeqLock *seqlock) +{ + uint32_t new_seq = seqlock->sequence - 1; + assert(!SEQLOCK_IS_UPDATING(new_seq)); + _Py_atomic_store_uint32(&seqlock->sequence, new_seq); +} + +void _PySeqLock_UnlockWrite(_PySeqLock *seqlock) +{ + uint32_t new_seq = seqlock->sequence + 1; + assert(!SEQLOCK_IS_UPDATING(new_seq)); + _Py_atomic_store_uint32(&seqlock->sequence, new_seq); +} + +uint32_t _PySeqLock_BeginRead(_PySeqLock *seqlock) +{ + uint32_t sequence = _Py_atomic_load_uint32_acquire(&seqlock->sequence); + while (SEQLOCK_IS_UPDATING(sequence)) { + _Py_yield(); + sequence = _Py_atomic_load_uint32_acquire(&seqlock->sequence); + } + + return sequence; +} + +uint32_t _PySeqLock_EndRead(_PySeqLock *seqlock, uint32_t previous) +{ + // Synchronize again and validate that the entry hasn't been updated + // while we were readying the values. + if (_Py_atomic_load_uint32_acquire(&seqlock->sequence) == previous) { + return 1; + } + + _Py_yield(); + return 0; +} + +uint32_t _PySeqLock_AfterFork(_PySeqLock *seqlock) +{ + // Synchronize again and validate that the entry hasn't been updated + // while we were readying the values. + if (SEQLOCK_IS_UPDATING(seqlock->sequence)) { + seqlock->sequence = 0; + return 1; + } + + return 0; +} diff --git a/Python/pystate.c b/Python/pystate.c index 08ec586963c..b1d1a085d62 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -395,6 +395,7 @@ _Py_COMP_DIAG_POP &(runtime)->atexit.mutex, \ &(runtime)->audit_hooks.mutex, \ &(runtime)->allocators.mutex, \ + &(runtime)->types.type_mutex, \ } static void @@ -499,6 +500,8 @@ _PyRuntimeState_ReInitThreads(_PyRuntimeState *runtime) _PyMutex_at_fork_reinit(locks[i]); } + _PyTypes_AfterFork(); + /* bpo-42540: id_mutex is freed by _PyInterpreterState_Delete, which does * not force the default allocator. */ if (_PyThread_at_fork_reinit(&runtime->interpreters.main->id_mutex) < 0) {