Skip to content

Commit dee63cb

Browse files
authored
gh-120860: Fix a few bugs in type_setattro error paths. (#120861)
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.
1 parent 0153fd0 commit dee63cb

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
@@ -5656,6 +5656,42 @@ _Py_type_getattro(PyObject *type, PyObject *name)
56565656
return _Py_type_getattro_impl((PyTypeObject *)type, name, NULL);
56575657
}
56585658

5659+
static int
5660+
type_update_dict(PyTypeObject *type, PyDictObject *dict, PyObject *name,
5661+
PyObject *value, PyObject **old_value)
5662+
{
5663+
// We don't want any re-entrancy between when we update the dict
5664+
// and call type_modified_unlocked, including running the destructor
5665+
// of the current value as it can observe the cache in an inconsistent
5666+
// state. Because we have an exact unicode and our dict has exact
5667+
// unicodes we know that this will all complete without releasing
5668+
// the locks.
5669+
if (_PyDict_GetItemRef_Unicode_LockHeld(dict, name, old_value) < 0) {
5670+
return -1;
5671+
}
5672+
5673+
/* Clear the VALID_VERSION flag of 'type' and all its
5674+
subclasses. This could possibly be unified with the
5675+
update_subclasses() recursion in update_slot(), but carefully:
5676+
they each have their own conditions on which to stop
5677+
recursing into subclasses. */
5678+
type_modified_unlocked(type);
5679+
5680+
if (_PyDict_SetItem_LockHeld(dict, name, value) < 0) {
5681+
PyErr_Format(PyExc_AttributeError,
5682+
"type object '%.50s' has no attribute '%U'",
5683+
((PyTypeObject*)type)->tp_name, name);
5684+
_PyObject_SetAttributeErrorContext((PyObject *)type, name);
5685+
return -1;
5686+
}
5687+
5688+
if (is_dunder_name(name)) {
5689+
return update_slot(type, name);
5690+
}
5691+
5692+
return 0;
5693+
}
5694+
56595695
static int
56605696
type_setattro(PyObject *self, PyObject *name, PyObject *value)
56615697
{
@@ -5698,12 +5734,11 @@ type_setattro(PyObject *self, PyObject *name, PyObject *value)
56985734
assert(!_PyType_HasFeature(metatype, Py_TPFLAGS_INLINE_VALUES));
56995735
assert(!_PyType_HasFeature(metatype, Py_TPFLAGS_MANAGED_DICT));
57005736

5701-
PyObject *old_value;
5737+
PyObject *old_value = NULL;
57025738
PyObject *descr = _PyType_LookupRef(metatype, name);
57035739
if (descr != NULL) {
57045740
descrsetfunc f = Py_TYPE(descr)->tp_descr_set;
57055741
if (f != NULL) {
5706-
old_value = NULL;
57075742
res = f(descr, (PyObject *)type, value);
57085743
goto done;
57095744
}
@@ -5719,47 +5754,16 @@ type_setattro(PyObject *self, PyObject *name, PyObject *value)
57195754
}
57205755
END_TYPE_LOCK();
57215756
if (dict == NULL) {
5722-
return -1;
5757+
res = -1;
5758+
goto done;
57235759
}
57245760
}
57255761

5726-
// We don't want any re-entrancy between when we update the dict
5727-
// and call type_modified_unlocked, including running the destructor
5728-
// of the current value as it can observe the cache in an inconsistent
5729-
// state. Because we have an exact unicode and our dict has exact
5730-
// unicodes we know that this will all complete without releasing
5731-
// the locks.
57325762
BEGIN_TYPE_DICT_LOCK(dict);
5733-
5734-
if (_PyDict_GetItemRef_Unicode_LockHeld((PyDictObject *)dict, name, &old_value) < 0) {
5735-
return -1;
5736-
}
5737-
5738-
/* Clear the VALID_VERSION flag of 'type' and all its
5739-
subclasses. This could possibly be unified with the
5740-
update_subclasses() recursion in update_slot(), but carefully:
5741-
they each have their own conditions on which to stop
5742-
recursing into subclasses. */
5743-
type_modified_unlocked(type);
5744-
5745-
res = _PyDict_SetItem_LockHeld((PyDictObject *)dict, name, value);
5746-
5747-
if (res == 0) {
5748-
if (is_dunder_name(name)) {
5749-
res = update_slot(type, name);
5750-
}
5751-
}
5752-
else if (PyErr_ExceptionMatches(PyExc_KeyError)) {
5753-
PyErr_Format(PyExc_AttributeError,
5754-
"type object '%.50s' has no attribute '%U'",
5755-
((PyTypeObject*)type)->tp_name, name);
5756-
5757-
_PyObject_SetAttributeErrorContext((PyObject *)type, name);
5758-
}
5759-
5763+
res = type_update_dict(type, (PyDictObject *)dict, name, value, &old_value);
57605764
assert(_PyType_CheckConsistency(type));
5761-
57625765
END_TYPE_DICT_LOCK();
5766+
57635767
done:
57645768
Py_DECREF(name);
57655769
Py_XDECREF(descr);

0 commit comments

Comments
 (0)