Index: Lib/test/test_descr.py =================================================================== --- Lib/test/test_descr.py (revision 67240) +++ Lib/test/test_descr.py (working copy) @@ -4289,7 +4289,47 @@ 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): class C(object): Index: Objects/typeobject.c =================================================================== --- Objects/typeobject.c (revision 67240) +++ Objects/typeobject.c (working copy) @@ -5391,6 +5391,25 @@ } static PyObject * +call_attribute(PyObject *attr, PyObject *self, PyObject *name) +{ + PyObject *res; + PyObject *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) { PyTypeObject *tp = Py_TYPE(self); @@ -5409,24 +5428,37 @@ if (getattribute_str == NULL) return NULL; } + /* speed hack: we could use lookup_maybe, but that would create + a method 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 create + a method 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); + Py_XINCREF(getattribute); 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); + res = call_attribute(getattribute, self, name); + Py_XDECREF(getattribute); if (res == NULL && PyErr_ExceptionMatches(PyExc_AttributeError)) { PyErr_Clear(); - res = PyObject_CallFunctionObjArgs(getattr, self, name, NULL); + res = call_attribute(getattr, self, name); } + Py_DECREF(getattr); return res; }