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).

diff --git a/Include/classobject.h b/Include/classobject.h
index 16af37beed910a0e054cdfe7a10cbecb2c449925..7c69964f09c6df0b62a652b76af3af57a79c6cce 100644 (file)
--- 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 4dd1590333288eb9da47a531dd4bb0a88237f9b1..81b6040e1708f4434f9bf96a26795e764824256d 100644 (file)
--- 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;