From 539afe0d8412551503e75b20a1f010560aa5fbc1 Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Tue, 8 Apr 2003 19:13:14 +0000 Subject: More backporting of gc-vs-__del__ fixes. It should be fixed for instances of classic classes now. Alas, new-style classes are still a problem, and didn't need to be fixed in 2.3 (they were already immune in 2.3 due to the new-in-2.3 tp_del slot). --- Include/classobject.h | 2 +- Modules/gcmodule.c | 22 +++++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/Include/classobject.h b/Include/classobject.h index 16af37b..7c69964 100644 --- a/Include/classobject.h +++ b/Include/classobject.h @@ -61,7 +61,7 @@ extern DL_IMPORT(PyObject *) PyMethod_Class(PyObject *); * can't fail, never sets an exception, and NULL is not an error (it just * means "not found"). */ -PyAPI_FUNC(PyObject *) _PyInstance_Lookup(PyObject *pinst, PyObject *name); +extern DL_IMPORT(PyObject *) _PyInstance_Lookup(PyObject *pinst, PyObject *name); /* Macros for direct access to these values. Type checks are *not* done, so use with care. */ diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 4dd1590..81b6040 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -253,7 +253,14 @@ move_root_reachable(PyGC_Head *reachable) } } -/* return true if object has a finalization method */ +/* Return true if object has a finalization method. + * CAUTION: class instances have to be checked for a __del__ method, and + * earlier versions of this used to call PyObject_HasAttr, which in turn + * could call the class's __getattr__ hook (if any). That could invoke + * arbitrary Python code, mutating the object graph in arbitrary ways, and + * that was the source of some excruciatingly subtle bugs. + * XXX This is still broken for new-style classes. + */ static int has_finalizer(PyObject *op) { @@ -263,9 +270,14 @@ has_finalizer(PyObject *op) if (delstr == NULL) Py_FatalError("PyGC: can't initialize __del__ string"); } - return (PyInstance_Check(op) || - PyType_HasFeature(op->ob_type, Py_TPFLAGS_HEAPTYPE)) - && PyObject_HasAttr(op, delstr); + + if (PyInstance_Check(op)) + return _PyInstance_Lookup(op, delstr) != NULL; + else if (PyType_HasFeature(op->ob_type, Py_TPFLAGS_HEAPTYPE)) + /* XXX This path is still Evil. */ + return PyObject_HasAttr(op, delstr); + else + return 0; } /* Move all objects with finalizers (instances with __del__) */ @@ -276,7 +288,7 @@ move_finalizers(PyGC_Head *unreachable, PyGC_Head *finalizers) PyGC_Head *gc = unreachable->gc.gc_next; for (; gc != unreachable; gc=next) { PyObject *op = FROM_GC(gc); - /* has_finalizer() may result in arbitrary Python + /* XXX has_finalizer() may result in arbitrary Python code being run. */ if (has_finalizer(op)) { next = gc->gc.gc_next; -- cgit v0.12