summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDino Viehland <dinoviehland@meta.com>2024-05-06 17:50:35 (GMT)
committerGitHub <noreply@github.com>2024-05-06 17:50:35 (GMT)
commit5a1618a2c8c108b8c73aa9459b63f0dbd66b60f6 (patch)
treea9b79e4cb8e0a9d1d648e21619382a03363c1f46
parente6b213ee3ffb05f067d30cb8bb45681887212444 (diff)
downloadcpython-5a1618a2c8c108b8c73aa9459b63f0dbd66b60f6.zip
cpython-5a1618a2c8c108b8c73aa9459b63f0dbd66b60f6.tar.gz
cpython-5a1618a2c8c108b8c73aa9459b63f0dbd66b60f6.tar.bz2
gh-118362: Fix thread safety around lookups from the type cache in the face of concurrent mutators (#118454)
Add _PyType_LookupRef and use incref before setting attribute on type Makes setting an attribute on a class and signaling type modified atomic Avoid adding re-entrancy exposing the type cache in an inconsistent state by decrefing after type is updated
-rw-r--r--Include/cpython/object.h1
-rw-r--r--Include/cpython/pyatomic.h3
-rw-r--r--Include/cpython/pyatomic_gcc.h4
-rw-r--r--Include/cpython/pyatomic_msc.h13
-rw-r--r--Include/cpython/pyatomic_std.h8
-rw-r--r--Include/internal/pycore_dict.h3
-rw-r--r--Include/internal/pycore_object.h1
-rw-r--r--Lib/test/test_class.py19
-rw-r--r--Lib/test/test_free_threading/test_type.py112
-rw-r--r--Misc/NEWS.d/next/Core and Builtins/2024-05-01-14-20-28.gh-issue-118492.VUsSfn.rst2
-rw-r--r--Modules/_collectionsmodule.c6
-rw-r--r--Modules/_ctypes/_ctypes.c2
-rw-r--r--Modules/_lsprof.c3
-rw-r--r--Modules/_testcapi/gc.c3
-rw-r--r--Objects/classobject.c22
-rw-r--r--Objects/dictobject.c50
-rw-r--r--Objects/object.c40
-rw-r--r--Objects/typeobject.c269
18 files changed, 437 insertions, 124 deletions
diff --git a/Include/cpython/object.h b/Include/cpython/object.h
index c2830b7..e624326 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 69083f1..55a139b 100644
--- a/Include/cpython/pyatomic.h
+++ b/Include/cpython/pyatomic.h
@@ -485,6 +485,9 @@ 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);
static inline uint64_t
diff --git a/Include/cpython/pyatomic_gcc.h b/Include/cpython/pyatomic_gcc.h
index af78a94..c0f3747 100644
--- a/Include/cpython/pyatomic_gcc.h
+++ b/Include/cpython/pyatomic_gcc.h
@@ -517,6 +517,10 @@ _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 212cd78..f32995c 100644
--- a/Include/cpython/pyatomic_msc.h
+++ b/Include/cpython/pyatomic_msc.h
@@ -990,6 +990,19 @@ _Py_atomic_load_int_acquire(const int *obj)
}
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)
{
#if defined(_M_X64) || defined(_M_IX86)
diff --git a/Include/cpython/pyatomic_std.h b/Include/cpython/pyatomic_std.h
index 6a77eae..0cdce4e 100644
--- a/Include/cpython/pyatomic_std.h
+++ b/Include/cpython/pyatomic_std.h
@@ -912,6 +912,14 @@ _Py_atomic_load_int_acquire(const int *obj)
}
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)
{
_Py_USING_STD;
diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h
index f33026d..3ba8ee7 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 3b0222b..f15c332 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 5885db8..655d53b 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 0000000..76208c4
--- /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 0000000..57d0f17
--- /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 b865351..644a90a 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 1b1a0ea..574cb80 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 7a9041a..5cf9eba 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 f4feaaa..b472a41 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 9cbb944..69a7d5f 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 1f21f70..3e662e0 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 1bf0e65..effbd51 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 ec19a3d..4b144fa 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 {
+ // If we can't incref the object we need to fallback to locking
+ break;
}
- } else {
+ }
+ 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
@@ -5162,6 +5211,14 @@ _PyType_Lookup(PyTypeObject *type, PyObject *name)
}
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)
{
PyObject *oname;
@@ -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);
+ if (!PyUnicode_Check(name)) {
+ PyErr_Format(PyExc_TypeError,
+ "attribute name must be string, not '%.200s'",
+ Py_TYPE(name)->tp_name);
+ return -1;
+ }
+
+ 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;
}
- else {
- name = _PyUnicode_Copy(name);
- if (name == NULL)
- 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;
}
- /* 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;
- }
+ }
+
+ 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;
}
}
- else {
- /* Will fail in _PyObject_GenericSetAttrWithDict. */
- Py_INCREF(name);
+
+ // 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;
}
- 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);
+#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);
+
+ if (res == 0) {
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;