Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions Lib/test/test_gc.py
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,76 @@ 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):
# 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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
10 changes: 4 additions & 6 deletions Modules/gcmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
Expand All @@ -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);
}

Expand Down