8000 gh-118362: Fix thread safety around lookups from the type cache in the face of concurrent mutators by DinoV · Pull Request #118454 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-118362: Fix thread safety around lookups from the type cache in the face of concurrent mutators #118454

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rename _PyType_Fetch -> _PyType_LookupRef
Fix some formatting
Add news blurb
Use PyDictObject * more
Rename _PyDict_GetItemRef_LockHeld to _PyDict_GetItemRef_Unicode_LockHeld
Reduce iterations in test
Expose _PyObject_SetAttributeErrorContext for attaching context
  • Loading branch information
DinoV committed May 1, 2024
commit 8074d0df528bfa3550ffc35922e2079a904a0cac
2 changes: 1 addition & 1 deletion Include/cpython/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ typedef struct _heaptypeobject {

PyAPI_FUNC(const char *) _PyType_Name(PyTypeObject *);
PyAPI_FUNC(PyObject *) _PyType_Lookup(PyTypeObject *, PyObject *);
PyAPI_FUNC(PyObject *) _PyType_Fetch(PyTypeObject *, PyObject *);
PyAPI_FUNC(PyObject *) _PyType_LookupRef(PyTypeObject *, PyObject *);
PyAPI_FUNC(PyObject *) PyType_GetDict(PyTypeObject *);

PyAPI_FUNC(int) PyObject_Print(PyObject *, FILE *, int);
Expand Down
4 changes: 2 additions & 2 deletions Include/internal/pycore_dict.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ PyAPI_FUNC(PyObject *)_PyDict_LoadGlobal(PyDictObject *, PyDictObject *, PyObjec
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_LockHeld(PyObject *op, PyObject *key, PyObject **result);
extern int _PyDict_GetItemRef_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash, PyObject **result);
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,
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
extern int _PyObject_SetAttributeErrorContext(PyObject* v, PyObject* name);
extern int _PyObject_SetAttributeErrorContext(PyObject *v, PyObject *name);


void _PyObject_InitInlineValues(PyObject *obj, PyTypeObject *tp);
extern int _PyObject_StoreInstanceAttribute(PyObject *obj,
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_free_threading/test_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class C:

DONE = False
def writer_func():
for i in range(10000):
for i in range(3000):
C.x
C.x
C.x += 1
Expand Down Expand Up @@ -75,7 +75,7 @@ class D(C):

DONE = False
def writer_func():
for i in range(10000):
for i in range(3000):
D.x
D.x
C.x += 1
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix an issue where the type cache can expose a previously accessed attribute
when a finalizer is run.
4 changes: 2 additions & 2 deletions Modules/_collectionsmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_Fetch(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_Fetch(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 &&
Expand Down
2 changes: 1 addition & 1 deletion Modules/_lsprof.c
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ normalizeUserObj(PyObject *obj)
PyObject *modname = fn->m_module;

if (name != NULL) {
PyObject *mo = _PyType_Fetch(Py_TYPE(self), name);
PyObject *mo = _PyType_LookupRef(Py_TYPE(self), name);
Py_DECREF(name);
if (mo != NULL) {
PyObject *res = PyObject_Repr(mo);
Expand Down
2 changes: 1 addition & 1 deletion Modules/_testcapi/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ slot_tp_del(PyObject *self)
return;
}
/* Execute __del__ method, if any. */
del = _PyType_Fetch(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);
Expand Down
4 changes: 2 additions & 2 deletions Objects/classobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ method_getattro(PyObject *obj, PyObject *name)
if (PyType_Ready(tp) < 0)
return NULL;
}
descr = _PyType_Fetch(tp, name);
descr = _PyType_LookupRef(tp, name);
}

if (descr != NULL) {
Expand Down Expand Up @@ -413,7 +413,7 @@ instancemethod_getattro(PyObject *self, PyObject *name)
if (PyType_Ready(tp) < 0)
return NULL;
}
descr = _PyType_Fetch(tp, name);
descr = _PyType_LookupRef(tp, name);

if (descr != NULL) {
descrgetfunc f = TP_DESCR_GET(Py_TYPE(descr));
Expand Down
16 changes: 6 additions & 10 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -2314,11 +2312,11 @@ 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_LockHeld(PyObject *op, PyObject *key, PyObject **result)
_PyDict_GetItemRef_Unicode_LockHeld(PyDictObject *op, PyObject *key, PyObject **result)
{
ASSERT_DICT_LOCKED(op);
assert(PyUnicode_CheckExact(key));
Expand All @@ -2332,10 +2330,8 @@ _PyDict_GetItemRef_LockHeld(PyObject *op, PyObject *key, PyObject **result)
}
}

PyDictObject*mp = (PyDictObject *)op;

PyObject *value;
Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &value);
Py_ssize_t ix = _Py_dict_lookup(op, key, hash, &value);
assert(ix >= 0 || value == NULL);
if (ix == DKIX_ERROR) {
*result = NULL;
Expand Down
22 changes: 11 additions & 11 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)){
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -1466,7 +1466,7 @@ _PyObject_GetMethod(PyObject *obj, PyObject *name, PyObject **method)
return 0;
}

PyObject *descr = _PyType_Fetch(tp, name);
PyObject *descr = _PyType_LookupRef(tp, name);
descrgetfunc f = NULL;
if (descr != NULL) {
if (_PyType_HasFeature(Py_TYPE(descr), Py_TPFLAGS_METHOD_DESCRIPTOR)) {
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -1569,7 +1569,7 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name,
goto done;
}

descr = _PyType_Fetch(tp, name);
descr = _PyType_LookupRef(tp, name);

f = NULL;
if (descr != NULL) {
Expand Down Expand Up @@ -1646,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);
Expand All @@ -1669,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'",
Expand All @@ -1682,7 +1683,7 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,

Py_INCREF(name);
Py_INCREF(tp);
descr = _PyType_Fetch(tp, name);
descr = _PyType_LookupRef(tp, name);

if (descr != NULL) {
f = Py_TYPE(descr)->tp_descr_set;
Expand Down Expand Up @@ -1720,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,
Expand All @@ -1743,11 +1744,10 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name,
}
error_check:
if (res < 0 && PyErr_ExceptionMatches(PyExc_KeyError)) {
assert(!PyType_IsSubtype(tp, &PyType_Type));
PyErr_Format(PyExc_AttributeError,
"'%.100s' object has no attribute '%U'",
tp->tp_name, name);
set_attribute_error_context(obj, name);
_PyObject_SetAttributeErrorContext(obj, name);
}
done:
Py_XDECREF(descr);
Expand Down
Loading
0