From e3a8ff2b59f8e6e43d7b13b37af816cb7e432a23 Mon Sep 17 00:00:00 2001 From: "Albert Eduardovich N." Date: Fri, 3 Apr 2026 13:15:30 +0300 Subject: [PATCH] improve trackref (#7375) * improve trackref - use `NoneType` from `types` since python 3.9 is no longer supported - use `monotonic` instead of `time` and fix flakiness of `get_oldest` * tracing GC is no joke * refine tests * explain the tests * add note for pypy in docs + ... return empty tuple instead of list in `iter_all` --- scrapy/utils/trackref.py | 19 +++++++---- tests/test_utils_trackref.py | 62 ++++++++++++++++++++++++++++-------- 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/scrapy/utils/trackref.py b/scrapy/utils/trackref.py index b04214c51..082aca4b1 100644 --- a/scrapy/utils/trackref.py +++ b/scrapy/utils/trackref.py @@ -7,13 +7,19 @@ subclass from object_ref (instead of object). About performance: This library has a minimal performance impact when enabled, and no performance penalty at all when disabled (as object_ref becomes just an alias to object in that case). + +.. note:: PyPy uses a tracing garbage collector, so objects may + remain in the ``live_refs`` longer than expected, even after they + go out of scope. If deterministic behavior is required, you may need + to explicitly trigger garbage collection or call ``trackref.live_refs.clear()``. """ from __future__ import annotations from collections import defaultdict from operator import itemgetter -from time import time +from time import monotonic_ns +from types import NoneType from typing import TYPE_CHECKING, Any from weakref import WeakKeyDictionary @@ -24,7 +30,6 @@ if TYPE_CHECKING: from typing_extensions import Self -NoneType = type(None) live_refs: defaultdict[type, WeakKeyDictionary] = defaultdict(WeakKeyDictionary) @@ -35,7 +40,7 @@ class object_ref: def __new__(cls, *args: Any, **kwargs: Any) -> Self: obj = object.__new__(cls) - live_refs[cls][obj] = time() + live_refs[cls][obj] = monotonic_ns() return obj @@ -43,14 +48,14 @@ class object_ref: def format_live_refs(ignore: Any = NoneType) -> str: """Return a tabular representation of tracked objects""" s = "Live References\n\n" - now = time() + now_ns = monotonic_ns() for cls, wdict in sorted(live_refs.items(), key=lambda x: x[0].__name__): if not wdict: continue if issubclass(cls, ignore): continue - oldest = min(wdict.values()) - s += f"{cls.__name__:<30} {len(wdict):6} oldest: {int(now - oldest)}s ago\n" + oldest_ns = min(wdict.values()) + s += f"{cls.__name__:<30} {len(wdict):6} oldest: {int((now_ns - oldest_ns) // 1e9)}s ago\n" return s @@ -74,4 +79,4 @@ def iter_all(class_name: str) -> Iterable[Any]: for cls, wdict in live_refs.items(): if cls.__name__ == class_name: return wdict.keys() - return [] + return () diff --git a/tests/test_utils_trackref.py b/tests/test_utils_trackref.py index 3967c3365..7a7a264d3 100644 --- a/tests/test_utils_trackref.py +++ b/tests/test_utils_trackref.py @@ -1,10 +1,11 @@ +import sys from io import StringIO -from time import sleep, time from unittest import mock import pytest from scrapy.utils import trackref +from scrapy.utils.python import garbage_collect class Foo(trackref.object_ref): @@ -63,24 +64,57 @@ Foo 1 oldest: 0s ago\n\n""" ) +_IS_PYPY = "PyPy" in sys.version + + def test_get_oldest(): - o1 = Foo() + """ + Verify that `get_oldest` returns the oldest live instance of a class. - o1_time = time() + The test runs in two passes to expose differences between: + - CPython (reference counting, immediate destruction) + - PyPy (tracing GC, delayed destruction) - o2 = Bar() + Since `trackref` relies on weak references, delayed GC on PyPy can leave + stale entries in `live_refs`, affecting results unless explicitly cleared. + """ - o3_time = time() - if o3_time <= o1_time: - sleep(0.01) - o3_time = time() - if o3_time <= o1_time: - pytest.skip("time.time is not precise enough") + def _delete_o1(): + """Delete `o1` and ensure it is actually collected on PyPy.""" + nonlocal o1 + del o1 - o3 = Foo() # noqa: F841 - assert trackref.get_oldest("Foo") is o1 - assert trackref.get_oldest("Bar") is o2 - assert trackref.get_oldest("XXX") is None + if _IS_PYPY: + # On PyPy, `del` only removes the local reference. The object may + # still exist until the GC runs, so we force a collection cycle. + garbage_collect() + + def _do_asserts(): + assert trackref.get_oldest("Foo") is o1 + assert trackref.get_oldest("Bar") is o2 + # Ensure the newer Foo is not incorrectly considered the oldest + assert trackref.get_oldest("Foo") is not o3 + assert trackref.get_oldest("XXX") is None + + o1, o2, o3 = Foo(), Bar(), Foo() + + _do_asserts() + + # Remove the oldest Foo instance; o3 should now become the oldest + _delete_o1() + assert trackref.get_oldest("Foo") is o3 + + # PyPy-specific behavior where stale references may persist + # unless the registry is explicitly cleared. + if _IS_PYPY: + trackref.live_refs.clear() + + o1, o2, o3 = Foo(), Bar(), Foo() + + _do_asserts() + + _delete_o1() + assert trackref.get_oldest("Foo") is o3 def test_iter_all():