diff --git a/Include/cpython/object.h b/Include/cpython/object.h index c2830b75e66..e624326693d 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -275,6 +275,7 @@ typedef struct _heaptypeobject { PyAPI_FUNC(const char *) _PyType_Name(PyTypeObject *); PyAPI_FUNC(PyObject *) _PyType_Lookup(PyTypeObject *, PyObject *); +PyAPI_FUNC(PyObject *) _PyType_LookupRef(PyTypeObject *, PyObject *); PyAPI_FUNC(PyObject *) PyType_GetDict(PyTypeObject *); PyAPI_FUNC(int) PyObject_Print(PyObject *, FILE *, int); diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h index 69083f1d9dd..55a139bb915 100644 --- a/Include/cpython/pyatomic.h +++ b/Include/cpython/pyatomic.h @@ -484,6 +484,9 @@ _Py_atomic_store_int_release(int *obj, int value); static inline int _Py_atomic_load_int_acquire(const int *obj); +static inline void +_Py_atomic_store_uint32_release(uint32_t *obj, uint32_t value); + static inline void _Py_atomic_store_uint64_release(uint64_t *obj, uint64_t value); diff --git a/Include/cpython/pyatomic_gcc.h b/Include/cpython/pyatomic_gcc.h index af78a94c736..c0f3747be45 100644 --- a/Include/cpython/pyatomic_gcc.h +++ b/Include/cpython/pyatomic_gcc.h @@ -516,6 +516,10 @@ static inline int _Py_atomic_load_int_acquire(const int *obj) { return __atomic_load_n(obj, __ATOMIC_ACQUIRE); } +static inline void +_Py_atomic_store_uint32_release(uint32_t *obj, uint32_t value) +{ __atomic_store_n(obj, value, __ATOMIC_RELEASE); } + static inline void _Py_atomic_store_uint64_release(uint64_t *obj, uint64_t value) { __atomic_store_n(obj, value, __ATOMIC_RELEASE); } diff --git a/Include/cpython/pyatomic_msc.h b/Include/cpython/pyatomic_msc.h index 212cd7817d0..f32995c1f57 100644 --- a/Include/cpython/pyatomic_msc.h +++ b/Include/cpython/pyatomic_msc.h @@ -989,6 +989,19 @@ _Py_atomic_load_int_acquire(const int *obj) #endif } +static inline void +_Py_atomic_store_uint32_release(uint32_t *obj, uint32_t value) +{ +#if defined(_M_X64) || defined(_M_IX86) + *(uint32_t volatile *)obj = value; +#elif defined(_M_ARM64) + _Py_atomic_ASSERT_ARG_TYPE(unsigned __int32); + __stlr32((unsigned __int32 volatile *)obj, (unsigned __int32)value); +#else +# error "no implementation of _Py_atomic_store_uint32_release" +#endif +} + static inline void _Py_atomic_store_uint64_release(uint64_t *obj, uint64_t value) { diff --git a/Include/cpython/pyatomic_std.h b/Include/cpython/pyatomic_std.h index 6a77eae536d..0cdce4e6dd3 100644 --- a/Include/cpython/pyatomic_std.h +++ b/Include/cpython/pyatomic_std.h @@ -911,6 +911,14 @@ _Py_atomic_load_int_acquire(const int *obj) memory_order_acquire); } +static inline void +_Py_atomic_store_uint32_release(uint32_t *obj, uint32_t value) +{ + _Py_USING_STD; + atomic_store_explicit((_Atomic(uint32_t)*)obj, value, + memory_order_release); +} + static inline void _Py_atomic_store_uint64_release(uint64_t *obj, uint64_t value) { diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index f33026dbd6b..3ba8ee74b4d 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -106,6 +106,9 @@ PyAPI_FUNC(PyObject *)_PyDict_LoadGlobal(PyDictObject *, PyDictObject *, PyObjec /* Consumes references to key and value */ PyAPI_FUNC(int) _PyDict_SetItem_Take2(PyDictObject *op, PyObject *key, PyObject *value); extern int _PyObjectDict_SetItem(PyTypeObject *tp, PyObject **dictptr, PyObject *name, PyObject *value); +extern int _PyDict_SetItem_LockHeld(PyDictObject *dict, PyObject *name, PyObject *value); +extern int _PyDict_GetItemRef_Unicode_LockHeld(PyDictObject *op, PyObject *key, PyObject **result); +extern int _PyDict_GetItemRef_KnownHash(PyDictObject *op, PyObject *key, Py_hash_t hash, PyObject **result); extern int _PyDict_Pop_KnownHash( PyDictObject *dict, diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 3b0222b05cb..f15c332a7b0 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -658,6 +658,7 @@ extern PyObject *_PyType_NewManagedObject(PyTypeObject *type); extern PyTypeObject* _PyType_CalculateMetaclass(PyTypeObject *, PyObject *); extern PyObject* _PyType_GetDocFromInternalDoc(const char *, const char *); extern PyObject* _PyType_GetTextSignatureFromInternalDoc(const char *, const char *, int); +extern int _PyObject_SetAttributeErrorContext(PyObject *v, PyObject* name); void _PyObject_InitInlineValues(PyObject *obj, PyTypeObject *tp); extern int _PyObject_StoreInstanceAttribute(PyObject *obj, diff --git a/Lib/test/test_class.py b/Lib/test/test_class.py index 5885db84b66..655d53b8d5b 100644 --- a/Lib/test/test_class.py +++ b/Lib/test/test_class.py @@ -882,6 +882,25 @@ class TestInlineValues(unittest.TestCase): f.a = 3 self.assertEqual(f.a, 3) + def test_store_attr_type_cache(self): + """Verifies that the type cache doesn't provide a value which is + inconsistent from the dict.""" + class X: + def __del__(inner_self): + v = C.a + self.assertEqual(v, C.__dict__['a']) + + class C: + a = X() + + # prime the cache + C.a + C.a + + # destructor shouldn't be able to see inconsisent state + C.a = X() + C.a = X() + if __name__ == '__main__': unittest.main() diff --git a/Lib/test/test_free_threading/test_type.py b/Lib/test/test_free_threading/test_type.py new file mode 100644 index 00000000000..76208c42fc8 --- /dev/null +++ b/Lib/test/test_free_threading/test_type.py @@ -0,0 +1,112 @@ +import unittest + +from threading import Thread +from multiprocessing.dummy import Pool +from unittest import TestCase + +from test.support import is_wasi + + +NTHREADS = 6 +BOTTOM = 0 +TOP = 1000 +ITERS = 100 + +class A: + attr = 1 + +@unittest.skipIf(is_wasi, "WASI has no threads.") +class TestType(TestCase): + def test_attr_cache(self): + def read(id0): + for _ in range(ITERS): + for _ in range(BOTTOM, TOP): + A.attr + + def write(id0): + for _ in range(ITERS): + for _ in range(BOTTOM, TOP): + # Make _PyType_Lookup cache hot first + A.attr + A.attr + x = A.attr + x += 1 + A.attr = x + + + with Pool(NTHREADS) as pool: + pool.apply_async(read, (1,)) + pool.apply_async(write, (1,)) + pool.close() + pool.join() + + def test_attr_cache_consistency(self): + class C: + x = 0 + + DONE = False + def writer_func(): + for i in range(3000): + C.x + C.x + C.x += 1 + nonlocal DONE + DONE = True + + def reader_func(): + while True: + # We should always see a greater value read from the type than the + # dictionary + a = C.__dict__['x'] + b = C.x + self.assertGreaterEqual(b, a) + + if DONE: + break + + self.run_one(writer_func, reader_func) + + def test_attr_cache_consistency_subclass(self): + class C: + x = 0 + + class D(C): + pass + + DONE = False + def writer_func(): + for i in range(3000): + D.x + D.x + C.x += 1 + nonlocal DONE + DONE = True + + def reader_func(): + while True: + # We should always see a greater value read from the type than the + # dictionary + a = C.__dict__['x'] + b = D.x + self.assertGreaterEqual(b, a) + + if DONE: + break + + self.run_one(writer_func, reader_func) + + def run_one(self, writer_func, reader_func): + writer = Thread(target=writer_func) + readers = [] + for x in range(30): + reader = Thread(target=reader_func) + readers.append(reader) + reader.start() + + writer.start() + writer.join() + for reader in readers: + reader.join() + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-01-14-20-28.gh-issue-118492.VUsSfn.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-01-14-20-28.gh-issue-118492.VUsSfn.rst new file mode 100644 index 00000000000..57d0f1730b8 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-01-14-20-28.gh-issue-118492.VUsSfn.rst @@ -0,0 +1,2 @@ +Fix an issue where the type cache can expose a previously accessed attribute +when a finalizer is run. diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index b865351c93d..644a90a8c71 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -2511,9 +2511,9 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping, /* Only take the fast path when get() and __setitem__() * have not been overridden. */ - mapping_get = _PyType_Lookup(Py_TYPE(mapping), &_Py_ID(get)); + mapping_get = _PyType_LookupRef(Py_TYPE(mapping), &_Py_ID(get)); dict_get = _PyType_Lookup(&PyDict_Type, &_Py_ID(get)); - mapping_setitem = _PyType_Lookup(Py_TYPE(mapping), &_Py_ID(__setitem__)); + mapping_setitem = _PyType_LookupRef(Py_TYPE(mapping), &_Py_ID(__setitem__)); dict_setitem = _PyType_Lookup(&PyDict_Type, &_Py_ID(__setitem__)); if (mapping_get != NULL && mapping_get == dict_get && @@ -2587,6 +2587,8 @@ _collections__count_elements_impl(PyObject *module, PyObject *mapping, } done: + Py_XDECREF(mapping_get); + Py_XDECREF(mapping_setitem); Py_DECREF(it); Py_XDECREF(key); Py_XDECREF(newval); diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 1b1a0ea549f..574cb801449 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -1096,7 +1096,7 @@ static int UnionType_setattro(PyObject *self, PyObject *key, PyObject *value) { /* XXX Should we disallow deleting _fields_? */ - if (-1 == PyObject_GenericSetAttr(self, key, value)) + if (-1 == PyType_Type.tp_setattro(self, key, value)) return -1; if (PyUnicode_Check(key) && diff --git a/Modules/_lsprof.c b/Modules/_lsprof.c index 7a9041ac079..5cf9eba243b 100644 --- a/Modules/_lsprof.c +++ b/Modules/_lsprof.c @@ -177,8 +177,7 @@ normalizeUserObj(PyObject *obj) PyObject *modname = fn->m_module; if (name != NULL) { - PyObject *mo = _PyType_Lookup(Py_TYPE(self), name); - Py_XINCREF(mo); + PyObject *mo = _PyType_LookupRef(Py_TYPE(self), name); Py_DECREF(name); if (mo != NULL) { PyObject *res = PyObject_Repr(mo); diff --git a/Modules/_testcapi/gc.c b/Modules/_testcapi/gc.c index f4feaaafbdc..b472a4185a9 100644 --- a/Modules/_testcapi/gc.c +++ b/Modules/_testcapi/gc.c @@ -99,10 +99,11 @@ slot_tp_del(PyObject *self) return; } /* Execute __del__ method, if any. */ - del = _PyType_Lookup(Py_TYPE(self), tp_del); + del = _PyType_LookupRef(Py_TYPE(self), tp_del); Py_DECREF(tp_del); if (del != NULL) { res = PyObject_CallOneArg(del, self); + Py_DECREF(del); if (res == NULL) PyErr_WriteUnraisable(del); else diff --git a/Objects/classobject.c b/Objects/classobject.c index 9cbb9442c60..69a7d5f046e 100644 --- a/Objects/classobject.c +++ b/Objects/classobject.c @@ -188,15 +188,18 @@ method_getattro(PyObject *obj, PyObject *name) if (PyType_Ready(tp) < 0) return NULL; } - descr = _PyType_Lookup(tp, name); + descr = _PyType_LookupRef(tp, name); } if (descr != NULL) { descrgetfunc f = TP_DESCR_GET(Py_TYPE(descr)); - if (f != NULL) - return f(descr, obj, (PyObject *)Py_TYPE(obj)); + if (f != NULL) { + PyObject *res = f(descr, obj, (PyObject *)Py_TYPE(obj)); + Py_DECREF(descr); + return res; + } else { - return Py_NewRef(descr); + return descr; } } @@ -410,14 +413,17 @@ instancemethod_getattro(PyObject *self, PyObject *name) if (PyType_Ready(tp) < 0) return NULL; } - descr = _PyType_Lookup(tp, name); + descr = _PyType_LookupRef(tp, name); if (descr != NULL) { descrgetfunc f = TP_DESCR_GET(Py_TYPE(descr)); - if (f != NULL) - return f(descr, self, (PyObject *)Py_TYPE(self)); + if (f != NULL) { + PyObject *res = f(descr, self, (PyObject *)Py_TYPE(self)); + Py_DECREF(descr); + return res; + } else { - return Py_NewRef(descr); + return descr; } } diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 1f21f70f149..3e662e09ea5 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2268,15 +2268,13 @@ _PyDict_GetItem_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash) * exception occurred. */ int -_PyDict_GetItemRef_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash, PyObject **result) +_PyDict_GetItemRef_KnownHash(PyDictObject *op, PyObject *key, Py_hash_t hash, PyObject **result) { - PyDictObject*mp = (PyDictObject *)op; - PyObject *value; #ifdef Py_GIL_DISABLED - Py_ssize_t ix = _Py_dict_lookup_threadsafe(mp, key, hash, &value); + Py_ssize_t ix = _Py_dict_lookup_threadsafe(op, key, hash, &value); #else - Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &value); + Py_ssize_t ix = _Py_dict_lookup(op, key, hash, &value); #endif assert(ix >= 0 || value == NULL); if (ix == DKIX_ERROR) { @@ -2314,9 +2312,38 @@ PyDict_GetItemRef(PyObject *op, PyObject *key, PyObject **result) } } - return _PyDict_GetItemRef_KnownHash(op, key, hash, result); + return _PyDict_GetItemRef_KnownHash((PyDictObject *)op, key, hash, result); } +int +_PyDict_GetItemRef_Unicode_LockHeld(PyDictObject *op, PyObject *key, PyObject **result) +{ + ASSERT_DICT_LOCKED(op); + assert(PyUnicode_CheckExact(key)); + + Py_hash_t hash; + if ((hash = unicode_get_hash(key)) == -1) { + hash = PyObject_Hash(key); + if (hash == -1) { + *result = NULL; + return -1; + } + } + + PyObject *value; + Py_ssize_t ix = _Py_dict_lookup(op, key, hash, &value); + assert(ix >= 0 || value == NULL); + if (ix == DKIX_ERROR) { + *result = NULL; + return -1; + } + if (value == NULL) { + *result = NULL; + return 0; // missing key + } + *result = Py_NewRef(value); + return 1; // key is present +} /* Variant of PyDict_GetItem() that doesn't suppress exceptions. This returns NULL *with* an exception set if an exception occurred. @@ -6704,8 +6731,8 @@ exit: return dict; } -static int -set_or_del_lock_held(PyDictObject *dict, PyObject *name, PyObject *value) +int +_PyDict_SetItem_LockHeld(PyDictObject *dict, PyObject *name, PyObject *value) { if (value == NULL) { Py_hash_t hash; @@ -6767,7 +6794,7 @@ store_instance_attr_lock_held(PyObject *obj, PyDictValues *values, // so that no one else will see it. dict = make_dict_from_instance_attributes(PyInterpreterState_Get(), keys, values); if (dict == NULL || - set_or_del_lock_held(dict, name, value) < 0) { + _PyDict_SetItem_LockHeld(dict, name, value) < 0) { Py_XDECREF(dict); return -1; } @@ -6779,7 +6806,7 @@ store_instance_attr_lock_held(PyObject *obj, PyDictValues *values, _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(dict); - res = set_or_del_lock_held (dict, name, value); + res = _PyDict_SetItem_LockHeld(dict, name, value); return res; } @@ -6822,7 +6849,7 @@ store_instance_attr_dict(PyObject *obj, PyDictObject *dict, PyObject *name, PyOb res = store_instance_attr_lock_held(obj, values, name, value); } else { - res = set_or_del_lock_held(dict, name, value); + res = _PyDict_SetItem_LockHeld(dict, name, value); } Py_END_CRITICAL_SECTION(); return res; @@ -7253,6 +7280,7 @@ _PyObjectDict_SetItem(PyTypeObject *tp, PyObject **dictptr, res = PyDict_SetItem(dict, key, value); } } + ASSERT_CONSISTENT(dict); return res; } diff --git a/Objects/object.c b/Objects/object.c index 1bf0e65ec60..effbd51991e 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1132,8 +1132,8 @@ _PyObject_SetAttrId(PyObject *v, _Py_Identifier *name, PyObject *w) return result; } -static inline int -set_attribute_error_context(PyObject* v, PyObject* name) +int +_PyObject_SetAttributeErrorContext(PyObject* v, PyObject* name) { assert(PyErr_Occurred()); if (!PyErr_ExceptionMatches(PyExc_AttributeError)){ @@ -1188,7 +1188,7 @@ PyObject_GetAttr(PyObject *v, PyObject *name) } if (result == NULL) { - set_attribute_error_context(v, name); + _PyObject_SetAttributeErrorContext(v, name); } return result; } @@ -1466,13 +1466,13 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method) return 0; } - PyObject *descr = _PyType_Lookup(tp, name); + PyObject *descr = _PyType_LookupRef(tp, name); descrgetfunc f = NULL; if (descr != NULL) { - Py_INCREF(descr); if (_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)) { meth_found = 1; - } else { + } + else { f = Py_TYPE(descr)->tp_descr_get; if (f != NULL && PyDescr_IsData(descr)) { *method = f(descr, obj, (PyObject *)Py_TYPE(obj)); @@ -1535,7 +1535,7 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method) "'%.100s' object has no attribute '%U'", tp->tp_name, name); - set_attribute_error_context(obj, name); + _PyObject_SetAttributeErrorContext(obj, name); return 0; } @@ -1569,11 +1569,10 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, goto done; } - descr = _PyType_Lookup(tp, name); + descr = _PyType_LookupRef(tp, name); f = NULL; if (descr != NULL) { - Py_INCREF(descr); f = Py_TYPE(descr)->tp_descr_get; if (f != NULL && PyDescr_IsData(descr)) { res = f(descr, obj, (PyObject *)Py_TYPE(obj)); @@ -1647,7 +1646,7 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, "'%.100s' object has no attribute '%U'", tp->tp_name, name); - set_attribute_error_context(obj, name); + _PyObject_SetAttributeErrorContext(obj, name); } done: Py_XDECREF(descr); @@ -1670,6 +1669,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, descrsetfunc f; int res = -1; + assert(!PyType_IsSubtype(tp, &PyType_Type)); if (!PyUnicode_Check(name)){ PyErr_Format(PyExc_TypeError, "attribute name must be string, not '%.200s'", @@ -1683,10 +1683,9 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, Py_INCREF(name); Py_INCREF(tp); - descr = _PyType_Lookup(tp, name); + descr = _PyType_LookupRef(tp, name); if (descr != NULL) { - Py_INCREF(descr); f = Py_TYPE(descr)->tp_descr_set; if (f != NULL) { res = f(descr, obj, value); @@ -1722,7 +1721,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, "'%.100s' object has no attribute '%U'", tp->tp_name, name); } - set_attribute_error_context(obj, name); + _PyObject_SetAttributeErrorContext(obj, name); } else { PyErr_Format(PyExc_AttributeError, @@ -1745,17 +1744,10 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, } error_check: if (res < 0 && PyErr_ExceptionMatches(PyExc_KeyError)) { - if (PyType_IsSubtype(tp, &PyType_Type)) { - PyErr_Format(PyExc_AttributeError, - "type object '%.50s' has no attribute '%U'", - ((PyTypeObject*)obj)->tp_name, name); - } - else { - PyErr_Format(PyExc_AttributeError, - "'%.100s' object has no attribute '%U'", - tp->tp_name, name); - } - set_attribute_error_context(obj, name); + PyErr_Format(PyExc_AttributeError, + "'%.100s' object has no attribute '%U'", + tp->tp_name, name); + _PyObject_SetAttributeErrorContext(obj, name); } done: Py_XDECREF(descr); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ec19a3d461f..4b144fab5de 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -71,6 +71,16 @@ class object "PyObject *" "&PyBaseObject_Type" _PyCriticalSection_End(&_cs); \ } +#define BEGIN_TYPE_DICT_LOCK(d) \ + { \ + _PyCriticalSection2 _cs; \ + _PyCriticalSection2_Begin(&_cs, TYPE_LOCK, \ + &_PyObject_CAST(d)->ob_mutex); \ + +#define END_TYPE_DICT_LOCK() \ + _PyCriticalSection2_End(&_cs); \ + } + #define ASSERT_TYPE_LOCK_HELD() \ _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(TYPE_LOCK) @@ -78,6 +88,8 @@ class object "PyObject *" "&PyBaseObject_Type" #define BEGIN_TYPE_LOCK() #define END_TYPE_LOCK() +#define BEGIN_TYPE_DICT_LOCK(d) +#define END_TYPE_DICT_LOCK() #define ASSERT_TYPE_LOCK_HELD() #endif @@ -728,7 +740,7 @@ _PyType_InitCache(PyInterpreterState *interp) assert(entry->name == NULL); entry->version = 0; - // Set to None so _PyType_Lookup() can use Py_SETREF(), + // Set to None so _PyType_LookupRef() can use Py_SETREF(), // rather than using slower Py_XSETREF(). entry->name = Py_None; entry->value = NULL; @@ -740,7 +752,7 @@ static unsigned int _PyType_ClearCache(PyInterpreterState *interp) { struct type_cache *cache = &interp->types.type_cache; - // Set to None, rather than NULL, so _PyType_Lookup() can + // Set to None, rather than NULL, so _PyType_LookupRef() can // use Py_SETREF() rather than using slower Py_XSETREF(). type_cache_clear(cache, Py_None); @@ -849,6 +861,44 @@ PyType_Unwatch(int watcher_id, PyObject* obj) return 0; } +#ifdef Py_GIL_DISABLED + +static void +type_modification_starting_unlocked(PyTypeObject *type) +{ + ASSERT_TYPE_LOCK_HELD(); + + /* Clear version tags on all types, but leave the valid + version tag intact. This prepares for a modification so that + any concurrent readers of the type cache will not see invalid + values. + */ + if (!_PyType_HasFeature(type, Py_TPFLAGS_VALID_VERSION_TAG)) { + return; + } + + PyObject *subclasses = lookup_tp_subclasses(type); + if (subclasses != NULL) { + assert(PyDict_CheckExact(subclasses)); + + Py_ssize_t i = 0; + PyObject *ref; + while (PyDict_Next(subclasses, &i, NULL, &ref)) { + PyTypeObject *subclass = type_from_ref(ref); + if (subclass == NULL) { + continue; + } + type_modification_starting_unlocked(subclass); + Py_DECREF(subclass); + } + } + + /* 0 is not a valid version tag */ + _Py_atomic_store_uint32_release(&type->tp_version_tag, 0); +} + +#endif + static void type_modified_unlocked(PyTypeObject *type) { @@ -882,7 +932,7 @@ type_modified_unlocked(PyTypeObject *type) if (subclass == NULL) { continue; } - PyType_Modified(subclass); + type_modified_unlocked(subclass); Py_DECREF(subclass); } } @@ -2352,7 +2402,7 @@ PyType_IsSubtype(PyTypeObject *a, PyTypeObject *b) Variants: - _PyObject_LookupSpecial() returns NULL without raising an exception - when the _PyType_Lookup() call fails; + when the _PyType_LookupRef() call fails; - lookup_maybe_method() and lookup_method() are internal routines similar to _PyObject_LookupSpecial(), but can return unbound PyFunction @@ -2365,13 +2415,12 @@ _PyObject_LookupSpecial(PyObject *self, PyObject *attr) { PyObject *res; - res = _PyType_Lookup(Py_TYPE(self), attr); + res = _PyType_LookupRef(Py_TYPE(self), attr); if (res != NULL) { descrgetfunc f; - if ((f = Py_TYPE(res)->tp_descr_get) == NULL) - Py_INCREF(res); - else - res = f(res, self, (PyObject *)(Py_TYPE(self))); + if ((f = Py_TYPE(res)->tp_descr_get) != NULL) { + Py_SETREF(res, f(res, self, (PyObject *)(Py_TYPE(self)))); + } } return res; } @@ -2388,7 +2437,7 @@ _PyObject_LookupSpecialId(PyObject *self, _Py_Identifier *attrid) static PyObject * lookup_maybe_method(PyObject *self, PyObject *attr, int *unbound) { - PyObject *res = _PyType_Lookup(Py_TYPE(self), attr); + PyObject *res = _PyType_LookupRef(Py_TYPE(self), attr); if (res == NULL) { return NULL; } @@ -2396,16 +2445,12 @@ lookup_maybe_method(PyObject *self, PyObject *attr, int *unbound) if (_PyType_HasFeature(Py_TYPE(res), Py_TPFLAGS_METHOD_DESCRIPTOR)) { /* Avoid temporary PyMethodObject */ *unbound = 1; - Py_INCREF(res); } else { *unbound = 0; descrgetfunc f = Py_TYPE(res)->tp_descr_get; - if (f == NULL) { - Py_INCREF(res); - } - else { - res = f(res, self, (PyObject *)(Py_TYPE(self))); + if (f != NULL) { + Py_SETREF(res, f(res, self, (PyObject *)(Py_TYPE(self)))); } } return res; @@ -4981,14 +5026,13 @@ find_name_in_mro(PyTypeObject *type, PyObject *name, int *error) PyObject *base = PyTuple_GET_ITEM(mro, i); PyObject *dict = lookup_tp_dict(_PyType_CAST(base)); assert(dict && PyDict_Check(dict)); - res = _PyDict_GetItem_KnownHash(dict, name, hash); - if (res != NULL) { - break; - } - if (PyErr_Occurred()) { + if (_PyDict_GetItemRef_KnownHash((PyDictObject *)dict, name, hash, &res) < 0) { *error = -1; goto done; } + if (res != NULL) { + break; + } } *error = 0; done: @@ -5030,8 +5074,6 @@ update_cache(struct type_cache_entry *entry, PyObject *name, unsigned int versio #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) @@ -5075,7 +5117,7 @@ _PyTypes_AfterFork(void) /* Internal API to look for a name through the MRO. This returns a borrowed reference, and doesn't set an exception! */ PyObject * -_PyType_Lookup(PyTypeObject *type, PyObject *name) +_PyType_LookupRef(PyTypeObject *type, PyObject *name) { PyObject *res; int error; @@ -5089,19 +5131,25 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) while (1) { int sequence = _PySeqLock_BeginRead(&entry->sequence); uint32_t entry_version = _Py_atomic_load_uint32_relaxed(&entry->version); - uint32_t type_version = _Py_atomic_load_uint32_relaxed(&type->tp_version_tag); + uint32_t type_version = _Py_atomic_load_uint32_acquire(&type->tp_version_tag); if (entry_version == type_version && _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; + if (value == NULL || _Py_TryIncref(value)) { + if (_PySeqLock_EndRead(&entry->sequence, sequence)) { + return value; + } + Py_XDECREF(value); } - } else { + else { + // If we can't incref the object we need to fallback to locking + break; + } + } + else { // cache miss break; } @@ -5112,6 +5160,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)); + Py_XINCREF(entry->value); return entry->value; } #endif @@ -5161,6 +5210,14 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name) return res; } +PyObject * +_PyType_Lookup(PyTypeObject *type, PyObject *name) +{ + PyObject *res = _PyType_LookupRef(type, name); + Py_XDECREF(res); + return res; +} + PyObject * _PyType_LookupId(PyTypeObject *type, _Py_Identifier *name) { @@ -5218,7 +5275,7 @@ _PyType_SetFlagsRecursive(PyTypeObject *self, unsigned long mask, unsigned long } /* This is similar to PyObject_GenericGetAttr(), - but uses _PyType_Lookup() instead of just looking in type->tp_dict. + but uses _PyType_LookupRef() instead of just looking in type->tp_dict. The argument suppress_missing_attribute is used to provide a fast path for hasattr. The possible values are: @@ -5254,10 +5311,9 @@ _Py_type_getattro_impl(PyTypeObject *type, PyObject *name, int * suppress_missin meta_get = NULL; /* Look for the attribute in the metatype */ - meta_attribute = _PyType_Lookup(metatype, name); + meta_attribute = _PyType_LookupRef(metatype, name); if (meta_attribute != NULL) { - Py_INCREF(meta_attribute); meta_get = Py_TYPE(meta_attribute)->tp_descr_get; if (meta_get != NULL && PyDescr_IsData(meta_attribute)) { @@ -5274,10 +5330,9 @@ _Py_type_getattro_impl(PyTypeObject *type, PyObject *name, int * suppress_missin /* No data descriptor found on metatype. Look in tp_dict of this * type and its bases */ - attribute = _PyType_Lookup(type, name); + attribute = _PyType_LookupRef(type, name); if (attribute != NULL) { /* Implement descriptor functionality, if any */ - Py_INCREF(attribute); descrgetfunc local_get = Py_TYPE(attribute)->tp_descr_get; Py_XDECREF(meta_attribute); @@ -5322,7 +5377,7 @@ _Py_type_getattro_impl(PyTypeObject *type, PyObject *name, int * suppress_missin } /* This is similar to PyObject_GenericGetAttr(), - but uses _PyType_Lookup() instead of just looking in type->tp_dict. */ + but uses _PyType_LookupRef() instead of just looking in type->tp_dict. */ PyObject * _Py_type_getattro(PyObject *type, PyObject *name) { @@ -5341,49 +5396,110 @@ type_setattro(PyObject *self, PyObject *name, PyObject *value) name, type->tp_name); return -1; } - if (PyUnicode_Check(name)) { - if (PyUnicode_CheckExact(name)) { - Py_INCREF(name); - } - else { - name = _PyUnicode_Copy(name); - if (name == NULL) - return -1; - } - /* bpo-40521: Interned strings are shared by all subinterpreters */ - if (!PyUnicode_CHECK_INTERNED(name)) { - PyUnicode_InternInPlace(&name); - if (!PyUnicode_CHECK_INTERNED(name)) { - PyErr_SetString(PyExc_MemoryError, - "Out of memory interning an attribute name"); - Py_DECREF(name); - return -1; - } - } + if (!PyUnicode_Check(name)) { + PyErr_Format(PyExc_TypeError, + "attribute name must be string, not '%.200s'", + Py_TYPE(name)->tp_name); + return -1; } - else { - /* Will fail in _PyObject_GenericSetAttrWithDict. */ + + if (PyUnicode_CheckExact(name)) { Py_INCREF(name); } + else { + name = _PyUnicode_Copy(name); + if (name == NULL) + return -1; + } + /* bpo-40521: Interned strings are shared by all subinterpreters */ + if (!PyUnicode_CHECK_INTERNED(name)) { + PyUnicode_InternInPlace(&name); + if (!PyUnicode_CHECK_INTERNED(name)) { + PyErr_SetString(PyExc_MemoryError, + "Out of memory interning an attribute name"); + Py_DECREF(name); + return -1; + } + } + + PyTypeObject *metatype = Py_TYPE(type); + assert(!_PyType_HasFeature(metatype, Py_TPFLAGS_INLINE_VALUES)); + assert(!_PyType_HasFeature(metatype, Py_TPFLAGS_MANAGED_DICT)); + + PyObject *old_value; + PyObject *descr = _PyType_LookupRef(metatype, name); + if (descr != NULL) { + descrsetfunc f = Py_TYPE(descr)->tp_descr_set; + if (f != NULL) { + old_value = NULL; + res = f(descr, (PyObject *)type, value); + goto done; + } + } + + PyObject *dict = type->tp_dict; + if (dict == NULL) { + // We don't just do PyType_Ready because we could already be readying + BEGIN_TYPE_LOCK(); + dict = type->tp_dict; + if (dict == NULL) { + dict = type->tp_dict = PyDict_New(); + } + END_TYPE_LOCK(); + if (dict == NULL) { + return -1; + } + } + + // We don't want any re-entrancy between when we update the dict + // and call type_modified_unlocked, including running the destructor + // of the current value as it can observe the cache in an inconsistent + // state. Because we have an exact unicode and our dict has exact + // unicodes we know that this will all complete without releasing + // the locks. + BEGIN_TYPE_DICT_LOCK(dict); + + if (_PyDict_GetItemRef_Unicode_LockHeld((PyDictObject *)dict, name, &old_value) < 0) { + return -1; + } + +#ifdef Py_GIL_DISABLED + // In free-threaded builds readers can race with the lock-free portion + // of the type cache and the assignment into the dict. We clear all of the + // versions initially so no readers will succeed in the lock-free case. + // They'll then block on the type lock until the update below is done. + type_modification_starting_unlocked(type); +#endif + + res = _PyDict_SetItem_LockHeld((PyDictObject *)dict, name, value); + + /* Clear the VALID_VERSION flag of 'type' and all its + subclasses. This could possibly be unified with the + update_subclasses() recursion in update_slot(), but carefully: + they each have their own conditions on which to stop + recursing into subclasses. */ + type_modified_unlocked(type); - BEGIN_TYPE_LOCK() - res = _PyObject_GenericSetAttrWithDict((PyObject *)type, name, value, NULL); if (res == 0) { - /* Clear the VALID_VERSION flag of 'type' and all its - subclasses. This could possibly be unified with the - update_subclasses() recursion in update_slot(), but carefully: - they each have their own conditions on which to stop - recursing into subclasses. */ - type_modified_unlocked(type); - if (is_dunder_name(name)) { res = update_slot(type, name); } - assert(_PyType_CheckConsistency(type)); } - END_TYPE_LOCK() + else if (PyErr_ExceptionMatches(PyExc_KeyError)) { + PyErr_Format(PyExc_AttributeError, + "type object '%.50s' has no attribute '%U'", + ((PyTypeObject*)type)->tp_name, name); + _PyObject_SetAttributeErrorContext((PyObject *)type, name); + } + + assert(_PyType_CheckConsistency(type)); + + END_TYPE_DICT_LOCK(); +done: Py_DECREF(name); + Py_XDECREF(descr); + Py_XDECREF(old_value); return res; } @@ -9343,33 +9459,33 @@ _Py_slot_tp_getattr_hook(PyObject *self, PyObject *name) /* speed hack: we could use lookup_maybe, but that would resolve the method fully for each attribute lookup for classes with __getattr__, even when the attribute is present. So we use - _PyType_Lookup and create the method only when needed, with + _PyType_LookupRef and create the method only when needed, with call_attribute. */ - getattr = _PyType_Lookup(tp, &_Py_ID(__getattr__)); + getattr = _PyType_LookupRef(tp, &_Py_ID(__getattr__)); if (getattr == NULL) { /* No __getattr__ hook: use a simpler dispatcher */ tp->tp_getattro = _Py_slot_tp_getattro; return _Py_slot_tp_getattro(self, name); } - Py_INCREF(getattr); /* speed hack: we could use lookup_maybe, but that would resolve the method fully for each attribute lookup for classes with __getattr__, even when self has the default __getattribute__ - method. So we use _PyType_Lookup and create the method only when + method. So we use _PyType_LookupRef and create the method only when needed, with call_attribute. */ - getattribute = _PyType_Lookup(tp, &_Py_ID(__getattribute__)); + getattribute = _PyType_LookupRef(tp, &_Py_ID(__getattribute__)); if (getattribute == NULL || (Py_IS_TYPE(getattribute, &PyWrapperDescr_Type) && ((PyWrapperDescrObject *)getattribute)->d_wrapped == (void *)PyObject_GenericGetAttr)) { + Py_XDECREF(getattribute); res = _PyObject_GenericGetAttrWithDict(self, name, NULL, 1); /* if res == NULL with no exception set, then it must be an AttributeError suppressed by us. */ if (res == NULL && !PyErr_Occurred()) { res = call_attribute(self, getattr, name); } - } else { - Py_INCREF(getattribute); + } + else { res = call_attribute(self, getattribute, name); Py_DECREF(getattribute); if (res == NULL && PyErr_ExceptionMatches(PyExc_AttributeError)) { @@ -9476,7 +9592,7 @@ slot_tp_descr_get(PyObject *self, PyObject *obj, PyObject *type) PyTypeObject *tp = Py_TYPE(self); PyObject *get; - get = _PyType_Lookup(tp, &_Py_ID(__get__)); + get = _PyType_LookupRef(tp, &_Py_ID(__get__)); if (get == NULL) { /* Avoid further slowdowns */ if (tp->tp_descr_get == slot_tp_descr_get) @@ -9488,7 +9604,9 @@ slot_tp_descr_get(PyObject *self, PyObject *obj, PyObject *type) if (type == NULL) type = Py_None; PyObject *stack[3] = {self, obj, type}; - return PyObject_Vectorcall(get, stack, 3, NULL); + PyObject *res = PyObject_Vectorcall(get, stack, 3, NULL); + Py_DECREF(get); + return res; } static int @@ -10383,6 +10501,7 @@ update_one_slot(PyTypeObject *type, pytype_slotdef *p) type->tp_flags &= ~Py_TPFLAGS_HAVE_VECTORCALL; } } + Py_DECREF(descr); } while ((++p)->offset == offset); if (specific && !use_generic) *ptr = specific;