Reworked move_finalizer_reachable() to create two distinct lists:

externally unreachable objects with finalizers, and externally unreachable
objects without finalizers reachable from such objects.  This allows us
to call has_finalizer() at most once per object, and so limit the pain of
nasty getattr hooks.  This fixes the failing "boom 2" example Jeremy
posted (a non-printing variant of which is now part of test_gc), via never
triggering the nasty part of its __getattr__ method.

1 files changed, 32 insertions, 4 deletions

diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
index 5ec87b9..e225881 100644
--- a/Lib/test/test_gc.py
+++ b/Lib/test/test_gc.py
@@ -253,21 +253,21 @@ def test_trashcan():
             v = {1: v, 2: Ouch()}
     gc.disable()
 
-class C:
+class Boom:
     def __getattr__(self, someattribute):
         del self.attr
         raise AttributeError
 
 def test_boom():
-    a = C()
-    b = C()
+    a = Boom()
+    b = Boom()
     a.attr = b
     b.attr = a
 
     gc.collect()
     garbagelen = len(gc.garbage)
     del a, b
-    # a<->b are in a trash cycle now.  Collection will invoke C.__getattr__
+    # a<->b are in a trash cycle now.  Collection will invoke Boom.__getattr__
     # (to see whether a and b have __del__ methods), and __getattr__ deletes
     # the internal "attr" attributes as a side effect.  That causes the
     # trash cycle to get reclaimed via refcounts falling to 0, thus mutating
@@ -276,6 +276,33 @@ def test_boom():
     expect(gc.collect(), 0, "boom")
     expect(len(gc.garbage), garbagelen, "boom")
 
+class Boom2:
+    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():
+    a = Boom2()
+    b = Boom2()
+    a.attr = b
+    b.attr = a
+
+    gc.collect()
+    garbagelen = len(gc.garbage)
+    del a, b
+    # Much like test_boom(), except that __getattr__ doesn't break the
+    # cycle until the second time gc checks for __del__.  As of 2.3b1,
+    # there isn't a second time, so this simply cleans up the trash cycle.
+    # We expect a, b, a.__dict__ and b.__dict__ (4 objects) to get reclaimed
+    # this way.
+    expect(gc.collect(), 4, "boom2")
+    expect(len(gc.garbage), garbagelen, "boom2")
+
 def test_all():
     gc.collect() # Delete 2nd generation garbage
     run_test("lists", test_list)
@@ -295,6 +322,7 @@ def test_all():
     run_test("saveall", test_saveall)
     run_test("trashcan", test_trashcan)
     run_test("boom", test_boom)
+    run_test("boom2", test_boom2)
 
 def test():
     if verbose: