From: Pablo Galindo <Pablogsal@gmail.com>
Date: Wed, 9 Oct 2019 21:42:54 +0000 (+0100)
Subject: [3.7] bpo-38379:  don't claim objects are collected when they aren't (GH-16658) ... 
X-Git-Tag: v3.7.6rc1~117
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=a081e931505f190b5ccdff9a781e59b3f13fcc31;p=thirdparty%2FPython%2Fcpython.git

[3.7] bpo-38379:  don't claim objects are collected when they aren't (GH-16658) (GH-16685)



* [bpo-38379](https://bugs.python.org/issue38379):  when a finalizer resurrects an object,
nothing is actually collected in this run of gc.
Change the stats to relect that truth..
(cherry picked from commit ecbf35f9335b0420cb8adfda6f299d6747a16515)

Co-authored-by: Tim Peters <tim.peters@gmail.com>





https://bugs.python.org/issue38379



Automerge-Triggered-By: @pablogsal
---

diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
index 8d806db3ba57..a2fa8bbace4c 100644
--- a/Lib/test/test_gc.py
+++ b/Lib/test/test_gc.py
@@ -755,6 +755,77 @@ class GCTests(unittest.TestCase):
         gc.unfreeze()
         self.assertEqual(gc.get_freeze_count(), 0)
 
+    def test_38379(self):
+        # When a finalizer resurrects objects, stats were reporting them as
+        # having been collected.  This affected both collect()'s return
+        # value and the dicts returned by get_stats().
+        N = 100
+
+        class A:  # simple self-loop
+            def __init__(self):
+                self.me = self
+
+        class Z(A):  # resurrecting __del__
+            def __del__(self):
+                zs.append(self)
+
+        zs = []
+
+        def getstats():
+            d = gc.get_stats()[-1]
+            return d['collected'], d['uncollectable']
+
+        gc.collect()
+        gc.disable()
+
+        # No problems if just collecting A() instances.
+        oldc, oldnc = getstats()
+        for i in range(N):
+            A()
+        t = gc.collect()
+        c, nc = getstats()
+        self.assertEqual(t, 2*N) # instance object & its dict
+        self.assertEqual(c - oldc, 2*N)
+        self.assertEqual(nc - oldnc, 0)
+
+        # But Z() is not actually collected.
+        oldc, oldnc = c, nc
+        Z()
+        # Nothing is collected - Z() is merely resurrected.
+        t = gc.collect()
+        c, nc = getstats()
+        #self.assertEqual(t, 2)  # before
+        self.assertEqual(t, 0)  # after
+        #self.assertEqual(c - oldc, 2)   # before
+        self.assertEqual(c - oldc, 0)   # after
+        self.assertEqual(nc - oldnc, 0)
+
+        # Unfortunately, a Z() prevents _anything_ from being collected.
+        # It should be possible to collect the A instances anyway, but
+        # that will require non-trivial code changes.
+        oldc, oldnc = c, nc
+        for i in range(N):
+            A()
+        Z()
+        # Z() prevents anything from being collected.
+        t = gc.collect()
+        c, nc = getstats()
+        #self.assertEqual(t, 2*N + 2)  # before
+        self.assertEqual(t, 0)  # after
+        #self.assertEqual(c - oldc, 2*N + 2)   # before
+        self.assertEqual(c - oldc, 0)   # after
+        self.assertEqual(nc - oldnc, 0)
+
+        # But the A() trash is reclaimed on the next run.
+        oldc, oldnc = c, nc
+        t = gc.collect()
+        c, nc = getstats()
+        self.assertEqual(t, 2*N)
+        self.assertEqual(c - oldc, 2*N)
+        self.assertEqual(nc - oldnc, 0)
+
+        gc.enable()
+
 
 class GCCallbackTests(unittest.TestCase):
     def setUp(self):
diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-10-09-16-50-52.bpo-38379.oz5qZx.rst b/Misc/NEWS.d/next/Core and Builtins/2019-10-09-16-50-52.bpo-38379.oz5qZx.rst
new file mode 100644
index 000000000000..82dcb525dd49
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2019-10-09-16-50-52.bpo-38379.oz5qZx.rst	
@@ -0,0 +1 @@
+When cyclic garbage collection (gc) runs finalizers that resurrect unreachable objects, the current gc run ends, without collecting any cyclic trash.  However, the statistics reported by ``collect()`` and ``get_stats()`` claimed that all cyclic trash found was collected, and that the resurrected objects were collected.   Changed the stats to report that none were collected.
diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c
index 4d701cb72e8c..32586031c039 100644
--- a/Modules/gcmodule.c
+++ b/Modules/gcmodule.c
@@ -887,13 +887,9 @@ collect(int generation, Py_ssize_t *n_collected, Py_ssize_t *n_uncollectable,
      */
     move_legacy_finalizer_reachable(&finalizers);
 
-    /* Collect statistics on collectable objects found and print
-     * debugging information.
-     */
-    for (gc = unreachable.gc.gc_next; gc != &unreachable;
-                    gc = gc->gc.gc_next) {
-        m++;
-        if (_PyRuntime.gc.debug & DEBUG_COLLECTABLE) {
+    /* Print debugging information. */
+    if (_PyRuntime.gc.debug & DEBUG_COLLECTABLE) {
+        for (gc = unreachable.gc.gc_next; gc != &unreachable; gc = gc->gc.gc_next) {
             debug_cycle("collectable", FROM_GC(gc));
         }
     }
@@ -913,6 +909,7 @@ collect(int generation, Py_ssize_t *n_collected, Py_ssize_t *n_uncollectable,
          * the reference cycles to be broken.  It may also cause some objects
          * in finalizers to be freed.
          */
+        m += gc_list_size(&unreachable);
         delete_garbage(&unreachable, old);
     }