diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 735c9f09aa1..fa3e0c3b916 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -4001,6 +4001,46 @@ order (MRO) for bases """ c[1:2] = 3 self.assertEqual(c.value, 3) + def test_getattr_hooks(self): + # issue 4230 + + class Descriptor(object): + counter = 0 + def __get__(self, obj, objtype=None): + def getter(name): + self.counter += 1 + raise AttributeError(name) + return getter + + descr = Descriptor() + class A(object): + __getattribute__ = descr + class B(object): + __getattr__ = descr + class C(object): + __getattribute__ = descr + __getattr__ = descr + + self.assertRaises(AttributeError, getattr, A(), "attr") + self.assertEquals(descr.counter, 1) + self.assertRaises(AttributeError, getattr, B(), "attr") + self.assertEquals(descr.counter, 2) + self.assertRaises(AttributeError, getattr, C(), "attr") + self.assertEquals(descr.counter, 4) + + import gc + class EvilGetattribute(object): + # This used to segfault + def __getattr__(self, name): + raise AttributeError(name) + def __getattribute__(self, name): + del EvilGetattribute.__getattr__ + for i in range(5): + gc.collect() + raise AttributeError(name) + + self.assertRaises(AttributeError, getattr, EvilGetattribute(), "attr") + class DictProxyTests(unittest.TestCase): def setUp(self): diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 7f9551fa4c8..7f9a26dd0a1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4959,6 +4959,24 @@ slot_tp_getattro(PyObject *self, PyObject *name) "(O)", name); } +static PyObject * +call_attribute(PyObject *self, PyObject *attr, PyObject *name) +{ + PyObject *res, *descr = NULL; + descrgetfunc f = Py_TYPE(attr)->tp_descr_get; + + if (f != NULL) { + descr = f(attr, self, (PyObject *)(Py_TYPE(self))); + if (descr == NULL) + return NULL; + else + attr = descr; + } + res = PyObject_CallFunctionObjArgs(attr, name, NULL); + Py_XDECREF(descr); + return res; +} + static PyObject * slot_tp_getattr_hook(PyObject *self, PyObject *name) { @@ -4978,24 +4996,39 @@ slot_tp_getattr_hook(PyObject *self, PyObject *name) if (getattribute_str == NULL) return NULL; } + /* 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 + call_attribute. */ getattr = _PyType_Lookup(tp, getattr_str); if (getattr == NULL) { /* No __getattr__ hook: use a simpler dispatcher */ tp->tp_getattro = slot_tp_getattro; return 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 + needed, with call_attribute. */ getattribute = _PyType_Lookup(tp, getattribute_str); if (getattribute == NULL || (Py_TYPE(getattribute) == &PyWrapperDescr_Type && ((PyWrapperDescrObject *)getattribute)->d_wrapped == (void *)PyObject_GenericGetAttr)) res = PyObject_GenericGetAttr(self, name); - else - res = PyObject_CallFunctionObjArgs(getattribute, self, name, NULL); + else { + Py_INCREF(getattribute); + res = call_attribute(self, getattribute, name); + Py_DECREF(getattribute); + } if (res == NULL && PyErr_ExceptionMatches(PyExc_AttributeError)) { PyErr_Clear(); - res = PyObject_CallFunctionObjArgs(getattr, self, name, NULL); + res = call_attribute(self, getattr, name); } + Py_DECREF(getattr); return res; }