From 2b4e806d027231fc21e4ff22fff82ef0bd412c56 Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Tue, 8 Oct 2019 09:17:19 -0500 Subject: [PATCH 1/5] bpo-38379: when a finalizer resurrects an object, nothing is actually collected in this run of gc. Change the stats to relect that truth. --- Lib/test/test_gc.py | 54 +++++++++++++++++++++++++++++++++++++++++++++ Modules/gcmodule.c | 10 ++++----- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 8215390cb70896..161edb04a30d8f 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -822,6 +822,60 @@ def test_get_objects_arguments(self): self.assertRaises(TypeError, gc.get_objects, "1") self.assertRaises(TypeError, gc.get_objects, 1.234) + def test_38379(self): + 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() + + 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) + + 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) + + 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) + + gc.enable() class GCCallbackTests(unittest.TestCase): def setUp(self): diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c index 2b47abae1ae4d0..766f8e0c67fa8e 100644 --- a/Modules/gcmodule.c +++ b/Modules/gcmodule.c @@ -1095,12 +1095,9 @@ collect(struct _gc_runtime_state *state, int generation, validate_list(&finalizers, 0); validate_list(&unreachable, PREV_MASK_COLLECTING); - /* Collect statistics on collectable objects found and print - * debugging information. - */ - for (gc = GC_NEXT(&unreachable); gc != &unreachable; gc = GC_NEXT(gc)) { - m++; - if (state->debug & DEBUG_COLLECTABLE) { + /* Print debugging information. */ + if (state->debug & DEBUG_COLLECTABLE) { + for (gc = GC_NEXT(&unreachable); gc != &unreachable; gc = GC_NEXT(gc)) { debug_cycle("collectable", FROM_GC(gc)); } } @@ -1122,6 +1119,7 @@ collect(struct _gc_runtime_state *state, int generation, * the reference cycles to be broken. It may also cause some objects * in finalizers to be freed. */ + m += gc_list_size(&unreachable); delete_garbage(state, &unreachable, old); } From eb81d6cdec09566d5f6cafb3ea9d098ecbc28f48 Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Tue, 8 Oct 2019 09:58:46 -0500 Subject: [PATCH 2/5] Verify trash is reclaimed on the next gc run. --- Lib/test/test_gc.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 161edb04a30d8f..ba1e2ef01b8377 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -875,6 +875,14 @@ def getstats(): self.assertEqual(c - oldc, 0) # after self.assertEqual(nc - oldnc, 0) + # But the 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): From d005526eb0f6fdfd4b2495c5b0abaa9dae050d94 Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Tue, 8 Oct 2019 16:09:42 -0500 Subject: [PATCH 3/5] Add commenta to new test. --- Lib/test/test_gc.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index ba1e2ef01b8377..f52db1eab169cd 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -823,6 +823,9 @@ def test_get_objects_arguments(self): self.assertRaises(TypeError, gc.get_objects, 1.234) 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 @@ -842,6 +845,7 @@ def getstats(): gc.collect() gc.disable() + # No problems if just collecting A() instances. oldc, oldnc = getstats() for i in range(N): A() @@ -851,6 +855,7 @@ def getstats(): 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. @@ -862,6 +867,9 @@ def getstats(): 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() @@ -875,7 +883,7 @@ def getstats(): self.assertEqual(c - oldc, 0) # after self.assertEqual(nc - oldnc, 0) - # But the trash is reclaimed on the next run. + # But the A() trash is reclaimed on the next run. oldc, oldnc = c, nc t = gc.collect() c, nc = getstats() From f4278f45c74e96697e8dbae4e892567af228834e Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 9 Oct 2019 16:50:54 +0000 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../Core and Builtins/2019-10-09-16-50-52.bpo-38379.oz5qZx.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2019-10-09-16-50-52.bpo-38379.oz5qZx.rst 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 00000000000000..76e0cf0086470d --- /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. \ No newline at end of file From de68c3d599041f6998c3e66ddf776c7e3c0e4d17 Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Wed, 9 Oct 2019 11:53:11 -0500 Subject: [PATCH 5/5] Update 2019-10-09-16-50-52.bpo-38379.oz5qZx.rst fixed typo --- .../Core and Builtins/2019-10-09-16-50-52.bpo-38379.oz5qZx.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 76e0cf0086470d..82dcb525dd49d3 100644 --- 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 @@ -1 +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. \ No newline at end of file +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.