From 8998a819375d8aa439e560472edcee9d9fa5475c Mon Sep 17 00:00:00 2001
From: Tim Peters <tim.peters@gmail.com>
Date: Tue, 8 Apr 2003 20:33:05 +0000
Subject: Fixed the gc-vs-__del__ bugs for new-style classes.  That's it for
 this one.

---
 Lib/test/test_gc.py | 44 ++++++++++++++++++++++++++++++++++++++++++++
 Misc/NEWS           |  8 ++++++++
 Modules/gcmodule.c  | 16 ++++++----------
 3 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
index 3824d7c..3b8f415 100644
--- a/Lib/test/test_gc.py
+++ b/Lib/test/test_gc.py
@@ -257,6 +257,48 @@ def test_boom2():
     expect(gc.collect(), 4, "boom2")
     expect(len(gc.garbage), garbagelen, "boom2")
 
+# boom__new and boom2_new are exactly like boom and boom2, except use
+# new-style classes.
+
+class Boom_New(object):
+    def __getattr__(self, someattribute):
+        del self.attr
+        raise AttributeError
+
+def test_boom_new():
+    a = Boom_New()
+    b = Boom_New()
+    a.attr = b
+    b.attr = a
+
+    gc.collect()
+    garbagelen = len(gc.garbage)
+    del a, b
+    expect(gc.collect(), 4, "boom_new")
+    expect(len(gc.garbage), garbagelen, "boom_new")
+
+class Boom2_New(object):
+    def __init__(self):
+        self.x = 0
+
+    def __getattr__(self, someattribute):
+        self.x += 1
+        if self.x > 1:
+            del self.attr
+        raise AttributeError
+
+def test_boom2_new():
+    a = Boom2_New()
+    b = Boom2_New()
+    a.attr = b
+    b.attr = a
+
+    gc.collect()
+    garbagelen = len(gc.garbage)
+    del a, b
+    expect(gc.collect(), 4, "boom2_new")
+    expect(len(gc.garbage), garbagelen, "boom2_new")
+
 def test_all():
     gc.collect() # Delete 2nd generation garbage
     run_test("lists", test_list)
@@ -275,6 +317,8 @@ def test_all():
     run_test("trashcan", test_trashcan)
     run_test("boom", test_boom)
     run_test("boom2", test_boom2)
+    run_test("boom_new", test_boom_new)
+    run_test("boom2_new", test_boom2_new)
 
 def test():
     if verbose:
diff --git a/Misc/NEWS b/Misc/NEWS
index 77bb761..485b17d 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -2,6 +2,14 @@ What's New in Python 2.2.3 ?
 Release date: XX-XXX-2003
 ============================
 
+- Some horridly obscure problems were fixed involving interaction
+  between garbage collection and classes with "ambitious" getattr hooks.
+  If a class instance didn't have a __del__ method, but did have a
+  __getattr__ hook, and the instance became reachable only from an
+  unreachable cycle, and the hook resurrected or deleted unreachable
+  objects when asked to resolve "__del__", anything up to a segfault
+  could happen.  That's been repaired.
+
 - Skip locale test on Mac OS X.
 
 - Backported the "largefile" requirement for test_largefile on Mac OS X.
diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c
index 81b6040..f1edb72 100644
--- a/Modules/gcmodule.c
+++ b/Modules/gcmodule.c
@@ -259,7 +259,6 @@ move_root_reachable(PyGC_Head *reachable)
  * 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)
@@ -274,8 +273,7 @@ has_finalizer(PyObject *op)
 	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);
+		return _PyType_Lookup(op->ob_type, delstr) != NULL;
 	else
 		return 0;
 }
@@ -284,20 +282,18 @@ has_finalizer(PyObject *op)
 static void
 move_finalizers(PyGC_Head *unreachable, PyGC_Head *finalizers)
 {
-	PyGC_Head *next;
 	PyGC_Head *gc = unreachable->gc.gc_next;
-	for (; gc != unreachable; gc=next) {
+
+	while (gc != unreachable) {
+		PyGC_Head *next = gc->gc.gc_next;
 		PyObject *op = FROM_GC(gc);
-		/* XXX has_finalizer() may result in arbitrary Python
-		   code being run. */
+
 		if (has_finalizer(op)) {
-			next = gc->gc.gc_next;
 			gc_list_remove(gc);
 			gc_list_append(gc, finalizers);
 			gc->gc.gc_refs = GC_MOVED;
 		}
-		else
-			next = gc->gc.gc_next;
+		gc = next;
 	}
 }
 
-- 
cgit v0.12