From 1b34f0cf986c168a917f3e275e8ef730e8a10b2e Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 30 Oct 2026 21:47:14 -0400 Subject: [PATCH] fix(tests): let collect_until run a second collection collect_until ended on the first collection once its grace ran out. PyPy 7.3.19 ran an abandoned extractor's finaliser only after a second collection, so a slow first collection may have left the handle reported as still open (#2481). The cause is unverified; the fix matches the observed two-collection behaviour on PyPy. With a grace, collect_until cannot give up before MIN_COLLECTIONS_WITH_GRACE (2) collections. grace=0 is unchanged, as the existing helper test pins it. Adds a fake-clock test that fails without the floor. Refs #2481 --- tests/_support.py | 42 +++++++++---------- tests/foundation/test_extraction_ownership.py | 28 +++++++++++++ 2 files changed, 49 insertions(+), 21 deletions(-) diff --git a/tests/_support.py b/tests/_support.py index afe65efb98..9f10aa72a1 100644 --- a/tests/_support.py +++ b/tests/_support.py @@ -1171,38 +1171,38 @@ def close_extractor(extractor: object) -> None: _close_quietly(engine) -#: Seconds :func:`collect_until` keeps collecting for a finaliser after the first -#: :func:`gc.collect`. CPython runs the finaliser before ``collect`` returns, so it -#: gets no grace. GraalPy's ``collect`` forces a full Java GC, sleeps 15 ms "just -#: trying to give the GC more time" and then runs only the weak reference -#: callbacks queued by then (graalpython ``GcModuleBuiltins.javaCollect``, at -#: graal-25.4.4), so on a loaded runner the finaliser can run after it returns -#: (#1963). A finaliser that never runs still fails once the grace is spent. -#: PyPy frees an extractor, and so runs the finaliser, one collection later -#: than CPython: the class still defines ``__del__`` (#1958, measured on PyPy -#: 8.0.1), so a short grace lets a second collection run there. Shared by -#: every test that waits on a finaliser, a reader's buffer file among them -#: (#2134). +#: Seconds, counted from the call, that :func:`collect_until` keeps collecting for a +#: finaliser. CPython runs the finaliser before ``collect`` returns, so it gets no +#: grace. GraalPy's ``collect`` can return before a loaded runner has run the +#: finaliser (#1963). PyPy runs it one collection later than CPython, so the grace +#: gives the finaliser time to run there. A finaliser that never runs still fails +#: once the grace is spent. Shared by every test that waits on one (#2134). FINALISER_GRACE = {'graalpy': 10.0, 'pypy': 1.0}.get(sys.implementation.name, 0.0) +#: :func:`collect_until` cannot give up before this many :func:`gc.collect` calls +#: when given a grace. On PyPy 7.3.19 an abandoned extractor's finaliser ran only +#: after two (observed), so the grace is not spent on a first collection alone. +MIN_COLLECTIONS_WITH_GRACE = 2 -def collect_until(done: 'Callable[[], bool]', grace: float = FINALISER_GRACE) -> bool: - """Collect garbage until ``done()`` holds or ``grace`` seconds have passed. - - Args: - done: Zero-argument callable, checked after each collection. - grace: Seconds to keep collecting after the first collection. - Returns: - Whether ``done()`` held before the grace ran out. +def collect_until(done: 'Callable[[], bool]', grace: float = FINALISER_GRACE) -> bool: + """Collect garbage until ``done()`` holds or ``grace`` seconds pass from the call. + ``done`` is checked after each collection. With a grace, the wait cannot give + up before :data:`MIN_COLLECTIONS_WITH_GRACE` collections; with ``0``, one runs. + Returns whether ``done()`` held after one of them. """ deadline = time.monotonic() + grace + attempts = 0 while True: gc.collect() + attempts += 1 if done(): return True - if time.monotonic() >= deadline: + # NOTE: with a grace, a first collection that outlasts it must not end the + # wait: PyPy's finaliser needs a second one (#2481), so run that too. + enough = grace <= 0 or attempts >= MIN_COLLECTIONS_WITH_GRACE + if enough and time.monotonic() >= deadline: return False time.sleep(0.05) diff --git a/tests/foundation/test_extraction_ownership.py b/tests/foundation/test_extraction_ownership.py index 36e555444a..24dd18a6cd 100644 --- a/tests/foundation/test_extraction_ownership.py +++ b/tests/foundation/test_extraction_ownership.py @@ -529,6 +529,34 @@ def late() -> bool: # the first to finish past the deadline. self.assertEqual(len(collections), 5) + def test_collect_until_does_not_stop_on_a_slow_first_collection(self) -> None: + """A first collection longer than the grace still gets a second (#2481). + + PyPy's finaliser needs two collections. Each collection here costs two + fake seconds, more than the one-second grace, and the finaliser runs on + the second. Ending after the first would report it as never run. + + """ + clock = [0.0] + collections: 'list[None]' = [] + + def collect() -> int: + clock[0] += 2.0 + collections.append(None) + return 0 + + def sleep(seconds: float) -> None: + clock[0] += seconds + + def second() -> bool: + return len(collections) >= 2 + + with mock.patch.object(gc, 'collect', collect), \ + mock.patch.object(time, 'monotonic', lambda: clock[0]), \ + mock.patch.object(time, 'sleep', sleep): + self.assertTrue(collect_until(second, grace=1.0)) + self.assertEqual(len(collections), 2) + def test_finished_extraction_detaches_its_finaliser(self) -> None: """``_cleanup`` and the context manager unregister it, so nothing closes twice.""" import pcapkit