]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
try not to rely on GC working at all for tests
authorMike Bayer <mike_mp@zzzcomputing.com>
Tue, 28 Oct 2025 20:20:54 +0000 (16:20 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Tue, 28 Oct 2025 20:55:05 +0000 (16:55 -0400)
GC seems to be working in fewer cases across different interpreters,
so add a new step to force an InstanceState to act as though
it got dereferenced, for those cases where we have del'ed the object,
done a gc collect, and we know it should not be reachable.

Change-Id: Ie97af1d5739f8d5a24aabe1296f14ce99ec1b0e5
(cherry picked from commit 368f5ac6668456609cf5f862c0676108cef4bec8)

lib/sqlalchemy/orm/state.py
test/aaa_profiling/test_memusage.py
test/orm/test_hasparent.py

index d4bbf92099312ba929c23ca6233133bb8367db88..5c4155e09e95f5788b269f245ed729775d9c677a 100644 (file)
@@ -511,6 +511,31 @@ class InstanceState(interfaces.InspectionAttrInfo, Generic[_O]):
         # used by the test suite, apparently
         self._detach()
 
+    def _force_dereference(self) -> None:
+        """Force this InstanceState to act as though its weakref has
+        been GC'ed.
+
+        this is used for test code that has to test reactions to objects
+        being GC'ed.  We can't reliably force GCs to happen under all
+        CI circumstances.
+
+        """
+
+        # if _strong_obj is set, then our object would not be getting
+        # GC'ed (at least within the scope of what we use this for in tests).
+        # so make sure this is not set
+        assert self._strong_obj is None
+
+        obj = self.obj()
+        if obj is None:
+            # object was GC'ed and we're done!  woop
+            return
+
+        del obj
+
+        self._cleanup(self.obj)
+        self.obj = lambda: None  # type: ignore
+
     def _cleanup(self, ref: weakref.ref[_O]) -> None:
         """Weakref callback cleanup.
 
index e2f25d5d098e0ba8784c708c808b85e41f533bcc..4e067ea8d31e5888dd984427765eac2995f9cfcf 100644 (file)
@@ -1834,30 +1834,9 @@ class MiscMemoryIntensiveTests(fixtures.TestBase):
         s.commit()
 
 
-# these tests simply cannot run reliably on github actions machines.
-# even trying ten times, they sometimes can't proceed.
-@testing.add_to_marker.gc_intensive
 class WeakIdentityMapTest(_fixtures.FixtureTest):
     run_inserts = None
 
-    def run_up_to_n_times(self, fn, times):
-        error = None
-        for _ in range(times):
-            try:
-                fn()
-            except Exception as err:
-                error = err
-                with testing.db.begin() as conn:
-                    conn.execute(self.tables.addresses.delete())
-                    conn.execute(self.tables.users.delete())
-                continue
-            else:
-                break
-        else:
-            if error:
-                raise error
-
-    @testing.requires.predictable_gc
     def test_weakref(self):
         """test the weak-referencing identity map, which strongly-
         references modified items."""
@@ -1865,83 +1844,86 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
         users, User = self.tables.users, self.classes.User
         self.mapper_registry.map_imperatively(User, users)
 
-        def go():
-            with Session(testing.db) as s:
-                gc_collect()
+        with Session(testing.db) as s:
+            gc_collect()
 
-                s.add(User(name="ed"))
-                s.flush()
-                assert not s.dirty
+            s.add(User(name="ed"))
+            s.flush()
+            assert not s.dirty
 
-                user = s.query(User).one()
+            user = s.query(User).one()
 
-                # heisenberg the GC a little bit, since #7823 caused a lot more
-                # GC when mappings are set up, larger test suite started
-                # failing on this being gc'ed
-                user_is = user._sa_instance_state
-                del user
-                gc_collect()
-                gc_collect()
-                gc_collect()
-                assert user_is.obj() is None
+            user_is = user._sa_instance_state
+            del user
+            user_is._force_dereference()
 
-                assert len(s.identity_map) == 0
+            assert len(s.identity_map) == 0
 
-                user = s.query(User).one()
-                user.name = "fred"
-                del user
-                gc_collect()
-                assert len(s.identity_map) == 1
-                assert len(s.dirty) == 1
-                assert None not in s.dirty
-                s.flush()
-                gc_collect()
-                assert not s.dirty
-                assert not s.identity_map
+            user = s.query(User).one()
+            user_is = user._sa_instance_state
+            user.name = "fred"
 
-                user = s.query(User).one()
-                assert user.name == "fred"
-                assert s.identity_map
+            del user
+            gc_collect()
 
-        self.run_up_to_n_times(go, 10)
+            # object was not gc'ed
+            assert user_is.obj()
+
+            assert len(s.identity_map) == 1
+            assert len(s.dirty) == 1
+            assert None not in s.dirty
+            s.flush()
+            gc_collect()
+            user_is._force_dereference()
+
+            assert not s.dirty
+            assert not s.identity_map
+
+            user = s.query(User).one()
+            assert user.name == "fred"
+            assert s.identity_map
 
-    @testing.requires.predictable_gc
     def test_weakref_pickled(self):
         users, User = self.tables.users, pickleable.User
         self.mapper_registry.map_imperatively(User, users)
 
-        def go():
-            with Session(testing.db) as s:
-                gc_collect()
+        with Session(testing.db) as s:
+            gc_collect()
 
-                s.add(User(name="ed"))
-                s.flush()
-                assert not s.dirty
+            s.add(User(name="ed"))
+            s.flush()
+            assert not s.dirty
 
-                user = s.query(User).one()
-                user.name = "fred"
-                s.expunge(user)
+            user = s.query(User).one()
+            user.name = "fred"
+            s.expunge(user)
 
-                u2 = pickle.loads(pickle.dumps(user))
+            u2 = pickle.loads(pickle.dumps(user))
 
-                del user
-                s.add(u2)
+            uis = user._sa_instance_state
+            u2is = u2._sa_instance_state
 
-                del u2
-                gc_collect()
+            del user
+            uis._force_dereference()
 
-                assert len(s.identity_map) == 1
-                assert len(s.dirty) == 1
-                assert None not in s.dirty
-                s.flush()
-                gc_collect()
-                assert not s.dirty
+            s.add(u2)
 
-                assert not s.identity_map
+            del u2
 
-        self.run_up_to_n_times(go, 10)
+            # was not gced
+            assert u2is.obj() is not None
+            assert len(s.identity_map) == 1
+            assert len(s.dirty) == 1
+            assert None not in s.dirty
+            s.flush()
+
+            gc_collect()
+            u2is._force_dereference()
+
+            assert not s.dirty
+
+            assert not s.identity_map
 
-    @testing.requires.predictable_gc
     def test_weakref_with_cycles_o2m(self):
         Address, addresses, users, User = (
             self.classes.Address,
@@ -1956,43 +1938,42 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
             properties={"addresses": relationship(Address, backref="user")},
         )
         self.mapper_registry.map_imperatively(Address, addresses)
-        gc_collect()
 
-        def go():
-            with Session(testing.db) as s:
-                s.add(
-                    User(name="ed", addresses=[Address(email_address="ed1")])
-                )
-                s.commit()
+        with Session(testing.db) as s:
+            s.add(User(name="ed", addresses=[Address(email_address="ed1")]))
+            s.commit()
 
-                user = s.query(User).options(joinedload(User.addresses)).one()
-                user.addresses[0].user  # lazyload
-                eq_(
-                    user,
-                    User(name="ed", addresses=[Address(email_address="ed1")]),
-                )
+            user = s.query(User).options(joinedload(User.addresses)).one()
+            user.addresses[0].user  # lazyload
+            eq_(
+                user,
+                User(name="ed", addresses=[Address(email_address="ed1")]),
+            )
 
-                del user
-                gc_collect()
-                assert len(s.identity_map) == 0
+            del user
+            gc_collect()
+            assert len(s.identity_map) == 0
 
-                user = s.query(User).options(joinedload(User.addresses)).one()
-                user.addresses[0].email_address = "ed2"
-                user.addresses[0].user  # lazyload
-                del user
-                gc_collect()
-                assert len(s.identity_map) == 2
+            user = s.query(User).options(joinedload(User.addresses)).one()
+            uis = user._sa_instance_state
 
-                s.commit()
-                user = s.query(User).options(joinedload(User.addresses)).one()
-                eq_(
-                    user,
-                    User(name="ed", addresses=[Address(email_address="ed2")]),
-                )
+            user.addresses[0].email_address = "ed2"
+            user.addresses[0].user  # lazyload
+            del user
+            gc_collect()
 
-        self.run_up_to_n_times(go, 10)
+            # was not GC'ed
+            assert uis.obj() is not None
+
+            assert len(s.identity_map) == 2
+
+            s.commit()
+            user = s.query(User).options(joinedload(User.addresses)).one()
+            eq_(
+                user,
+                User(name="ed", addresses=[Address(email_address="ed2")]),
+            )
 
-    @testing.requires.predictable_gc
     def test_weakref_with_cycles_o2o(self):
         Address, addresses, users, User = (
             self.classes.Address,
@@ -2010,7 +1991,6 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
             },
         )
         self.mapper_registry.map_imperatively(Address, addresses)
-        gc_collect()
 
         s.add(User(name="ed", address=Address(email_address="ed1")))
         s.commit()
@@ -2019,16 +1999,29 @@ class WeakIdentityMapTest(_fixtures.FixtureTest):
         user.address.user
         eq_(user, User(name="ed", address=Address(email_address="ed1")))
 
+        uis = user._sa_instance_state
+        ais = user.address._sa_instance_state
         del user
         gc_collect()
+
+        # in case gc collect didn't work, force a dereference
+        uis._force_dereference()
+        ais._force_dereference()
+
+        # identity map is cleaned out
         assert len(s.identity_map) == 0
 
         user = s.query(User).options(joinedload(User.address)).one()
+        uis = user._sa_instance_state
         user.address.email_address = "ed2"
         user.address.user  # lazyload
 
         del user
         gc_collect()
+
+        # was not gc'ed
+        assert uis.obj() is not None
+
         assert len(s.identity_map) == 2
 
         s.commit()
index 6332a09f4c314524a770eee9538e77038ec2c569..98662b4004d4ad9367344b3369fc8fa0805459f9 100644 (file)
@@ -107,24 +107,22 @@ class ParentRemovalTest(fixtures.MappedTest):
 
         self._assert_not_hasparent(a1)
 
-    @testing.add_to_marker.gc_intensive
-    @testing.requires.predictable_gc
     def test_stale_state_positive_gc(self):
         User = self.classes.User
         s, u1, a1 = self._fixture()
 
         s.expunge(u1)
-        del u1
-        gc_collect()
+
+        u1_is = u1._sa_instance_state
+        # act as though GC happened
+        u1_is._force_dereference()
 
         u1 = s.query(User).first()
         u1.addresses.remove(a1)
 
         self._assert_not_hasparent(a1)
 
-    @testing.add_to_marker.gc_intensive
     @testing.requires.updateable_autoincrement_pks
-    @testing.requires.predictable_gc
     def test_stale_state_positive_pk_change(self):
         """Illustrate that we can't easily link a
         stale state to a fresh one if the fresh one has
@@ -139,9 +137,6 @@ class ParentRemovalTest(fixtures.MappedTest):
 
         s._expunge_states([attributes.instance_state(u1)])
 
-        del u1
-        gc_collect()
-
         u1 = s.query(User).first()
 
         # primary key change.  now we
@@ -169,8 +164,6 @@ class ParentRemovalTest(fixtures.MappedTest):
 
         self._assert_not_hasparent(a1)
 
-    @testing.add_to_marker.gc_intensive
-    @testing.requires.predictable_gc
     def test_stale_state_negative_child_expired(self):
         """illustrate the current behavior of
         expiration on the child.
@@ -189,15 +182,9 @@ class ParentRemovalTest(fixtures.MappedTest):
         u1.addresses.remove(a1)
 
         u2_is = u2._sa_instance_state
-        del u2
 
-        for i in range(5):
-            gc_collect()
-        # heisenberg the GC a little bit, since #7823 caused a lot more
-        # GC when mappings are set up, larger test suite started failing
-        # on this being gc'ed
-        o = u2_is.obj()
-        assert o is None
+        # act as though GC happened
+        u2_is._force_dereference()
 
         # controversy here.  The action is
         # to expire one object, not the other, and remove;
@@ -208,8 +195,6 @@ class ParentRemovalTest(fixtures.MappedTest):
         self._assert_not_hasparent(a1)
         # self._assert_hasparent(a1)
 
-    @testing.add_to_marker.gc_intensive
-    @testing.requires.predictable_gc
     def test_stale_state_negative(self):
         User = self.classes.User
         s, u1, a1 = self._fixture()
@@ -221,15 +206,8 @@ class ParentRemovalTest(fixtures.MappedTest):
         s._expunge_states([attributes.instance_state(u2)])
 
         u2_is = u2._sa_instance_state
-        del u2
-
-        for i in range(5):
-            gc_collect()
-        # heisenberg the GC a little bit, since #7823 caused a lot more
-        # GC when mappings are set up, larger test suite started failing
-        # on this being gc'ed
-        o = u2_is.obj()
-        assert o is None
+
+        u2_is._force_dereference()
 
         assert_raises_message(
             orm_exc.StaleDataError,