Skip to content

Commit 6aee5ed

Browse files
[3.13] gh-120860: Fix a few bugs in type_setattro error paths. (GH-120861) (#120963)
Moves the logic to update the type's dictionary to its own function in order to make the lock scoping more clear. Also, ensure that `name` is decref'd on the error path. (cherry picked from commit dee63cb) Co-authored-by: Sam Gross <[email protected]>
1 parent 9769b7a commit 6aee5ed

File tree

1 file changed

+41
-37
lines changed

1 file changed

+41
-37
lines changed

Objects/typeobject.c

+41-37
Original file line numberDiff line numberDiff line change
@@ -5465,6 +5465,42 @@ _Py_type_getattro(PyObject *type, PyObject *name)
54655465
return _Py_type_getattro_impl((PyTypeObject *)type, name, NULL);
54665466
}
54675467

5468+
static int
5469+
type_update_dict(PyTypeObject *type, PyDictObject *dict, PyObject *name,
5470+
PyObject *value, PyObject **old_value)
5471+
{
5472+
// We don't want any re-entrancy between when we update the dict
5473+
// and call type_modified_unlocked, including running the destructor
5474+
// of the current value as it can observe the cache in an inconsistent
5475+
// state. Because we have an exact unicode and our dict has exact
5476+
// unicodes we know that this will all complete without releasing
5477+
// the locks.
5478+
if (_PyDict_GetItemRef_Unicode_LockHeld(dict, name, old_value) < 0) {
5479+
return -1;
5480+
}
5481+
5482+
/* Clear the VALID_VERSION flag of 'type' and all its
5483+
subclasses. This could possibly be unified with the
5484+
update_subclasses() recursion in update_slot(), but carefully:
5485+
they each have their own conditions on which to stop
5486+
recursing into subclasses. */
5487+
type_modified_unlocked(type);
5488+
5489+
if (_PyDict_SetItem_LockHeld(dict, name, value) < 0) {
5490+
PyErr_Format(PyExc_AttributeError,
5491+
"type object '%.50s' has no attribute '%U'",
5492+
((PyTypeObject*)type)->tp_name, name);
5493+
_PyObject_SetAttributeErrorContext((PyObject *)type, name);
5494+
return -1;
5495+
}
5496+
5497+
if (is_dunder_name(name)) {
5498+
return update_slot(type, name);
5499+
}
5500+
5501+
return 0;
5502+
}
5503+
54685504
static int
54695505
type_setattro(PyObject *self, PyObject *name, PyObject *value)
54705506
{
@@ -5507,12 +5543,11 @@ type_setattro(PyObject *self, PyObject *name, PyObject *value)
55075543
assert(!_PyType_HasFeature(metatype, Py_TPFLAGS_INLINE_VALUES));
55085544
assert(!_PyType_HasFeature(metatype, Py_TPFLAGS_MANAGED_DICT));
55095545

5510-
PyObject *old_value;
5546+
PyObject *old_value = NULL;
55115547
PyObject *descr = _PyType_LookupRef(metatype, name);
55125548
if (descr != NULL) {
55135549
descrsetfunc f = Py_TYPE(descr)->tp_descr_set;
55145550
if (f != NULL) {
5515-
old_value = NULL;
55165551
res = f(descr, (PyObject *)type, value);
55175552
goto done;
55185553
}
@@ -5528,47 +5563,16 @@ type_setattro(PyObject *self, PyObject *name, PyObject *value)
55285563
}
55295564
END_TYPE_LOCK();
55305565
if (dict == NULL) {
5531-
return -1;
5566+
res = -1;
5567+
goto done;
55325568
}
55335569
}
55345570

5535-
// We don't want any re-entrancy between when we update the dict
5536-
// and call type_modified_unlocked, including running the destructor
5537-
// of the current value as it can observe the cache in an inconsistent
5538-
// state. Because we have an exact unicode and our dict has exact
5539-
// unicodes we know that this will all complete without releasing
5540-
// the locks.
55415571
BEGIN_TYPE_DICT_LOCK(dict);
5542-
5543-
if (_PyDict_GetItemRef_Unicode_LockHeld((PyDictObject *)dict, name, &old_value) < 0) {
5544-
return -1;
5545-
}
5546-
5547-
/* Clear the VALID_VERSION flag of 'type' and all its
5548-
subclasses. This could possibly be unified with the
5549-
update_subclasses() recursion in update_slot(), but carefully:
5550-
they each have their own conditions on which to stop
5551-
recursing into subclasses. */
5552-
type_modified_unlocked(type);
5553-
5554-
res = _PyDict_SetItem_LockHeld((PyDictObject *)dict, name, value);
5555-
5556-
if (res == 0) {
5557-
if (is_dunder_name(name)) {
5558-
res = update_slot(type, name);
5559-
}
5560-
}
5561-
else if (PyErr_ExceptionMatches(PyExc_KeyError)) {
5562-
PyErr_Format(PyExc_AttributeError,
5563-
"type object '%.50s' has no attribute '%U'",
5564-
((PyTypeObject*)type)->tp_name, name);
5565-
5566-
_PyObject_SetAttributeErrorContext((PyObject *)type, name);
5567-
}
5568-
5572+
res = type_update_dict(type, (PyDictObject *)dict, name, value, &old_value);
55695573
assert(_PyType_CheckConsistency(type));
5570-
55715574
END_TYPE_DICT_LOCK();
5575+
55725576
done:
55735577
Py_DECREF(name);
55745578
Py_XDECREF(descr);

0 commit comments

Comments
 (0)