GH-135552: Make the GC clear weakrefs later (GH-136189)

Fix a bug caused by the garbage collector clearing weakrefs too early.  The
weakrefs in the ``tp_subclasses`` dictionary are needed in order to correctly
invalidate type caches (for example, by calling ``PyType_Modified()``).
Clearing weakrefs before calling finalizers causes the caches to not be
correctly invalidated.  That can cause crashes since the caches can refer to
invalid objects.  Defer the clearing of weakrefs without callbacks until after
finalizers are executed.
This commit is contained in:
Neil Schemenauer 2025-08-07 16:32:17 -07:00 committed by GitHub
parent deb385a143
commit 350c58ba4e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 317 additions and 140 deletions

View file

@ -174,7 +174,7 @@ def test_simple(self):
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
self.assertIs(wr(), None)
self.assertIsNone(wr())
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
@ -188,12 +188,12 @@ def test_simple_resurrect(self):
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors(ids)
self.assertIsNot(wr(), None)
self.assertIsNotNone(wr())
self.clear_survivors()
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
self.assertIs(wr(), None)
self.assertIsNone(wr())
@support.cpython_only
def test_non_gc(self):
@ -265,7 +265,7 @@ def test_simple(self):
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
self.assertIs(wr(), None)
self.assertIsNone(wr())
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
@ -276,19 +276,24 @@ def test_simple_resurrect(self):
s = SelfCycleResurrector()
ids = [id(s)]
wr = weakref.ref(s)
wrc = weakref.ref(s, lambda x: None)
del s
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors(ids)
# XXX is this desirable?
self.assertIs(wr(), None)
# This used to be None because weakrefs were cleared before
# calling finalizers. Now they are cleared after.
self.assertIsNotNone(wr())
# A weakref with a callback is still cleared before calling
# finalizers.
self.assertIsNone(wrc())
# When trying to destroy the object a second time, __del__
# isn't called anymore (and the object isn't resurrected).
self.clear_survivors()
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
self.assertIs(wr(), None)
self.assertIsNone(wr())
def test_simple_suicide(self):
# Test the GC is able to deal with an object that kills its last
@ -301,11 +306,11 @@ def test_simple_suicide(self):
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
self.assertIs(wr(), None)
self.assertIsNone(wr())
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors([])
self.assertIs(wr(), None)
self.assertIsNone(wr())
class ChainedBase:
@ -378,18 +383,27 @@ def check_non_resurrecting_chain(self, classes):
def check_resurrecting_chain(self, classes):
N = len(classes)
def dummy_callback(ref):
pass
with SimpleBase.test():
nodes = self.build_chain(classes)
N = len(nodes)
ids = [id(s) for s in nodes]
survivor_ids = [id(s) for s in nodes if isinstance(s, SimpleResurrector)]
wrs = [weakref.ref(s) for s in nodes]
wrcs = [weakref.ref(s, dummy_callback) for s in nodes]
del nodes
gc.collect()
self.assert_del_calls(ids)
self.assert_survivors(survivor_ids)
# XXX desirable?
self.assertEqual([wr() for wr in wrs], [None] * N)
for wr in wrs:
# These values used to be None because weakrefs were cleared
# before calling finalizers. Now they are cleared after.
self.assertIsNotNone(wr())
for wr in wrcs:
# Weakrefs with callbacks are still cleared before calling
# finalizers.
self.assertIsNone(wr())
self.clear_survivors()
gc.collect()
self.assert_del_calls(ids)
@ -491,7 +505,7 @@ def test_legacy(self):
self.assert_del_calls(ids)
self.assert_tp_del_calls(ids)
self.assert_survivors([])
self.assertIs(wr(), None)
self.assertIsNone(wr())
gc.collect()
self.assert_del_calls(ids)
self.assert_tp_del_calls(ids)
@ -507,13 +521,13 @@ def test_legacy_resurrect(self):
self.assert_tp_del_calls(ids)
self.assert_survivors(ids)
# weakrefs are cleared before tp_del is called.
self.assertIs(wr(), None)
self.assertIsNone(wr())
self.clear_survivors()
gc.collect()
self.assert_del_calls(ids)
self.assert_tp_del_calls(ids * 2)
self.assert_survivors(ids)
self.assertIs(wr(), None)
self.assertIsNone(wr())
def test_legacy_self_cycle(self):
# Self-cycles with legacy finalizers end up in gc.garbage.
@ -527,11 +541,11 @@ def test_legacy_self_cycle(self):
self.assert_tp_del_calls([])
self.assert_survivors([])
self.assert_garbage(ids)
self.assertIsNot(wr(), None)
self.assertIsNotNone(wr())
# Break the cycle to allow collection
gc.garbage[0].ref = None
self.assert_garbage([])
self.assertIs(wr(), None)
self.assertIsNone(wr())
if __name__ == "__main__":