bpo-44895: libregrtest: refleak check clears types later (GH-28113)

libregrtest now clears the type cache later to reduce the risk of
false alarm when checking for reference leaks. Previously, the type
cache was cleared too early and libregrtest raised a false alarm
about reference leaks under very specific conditions.

Move also support.gc_collect() outside clear/cleanup functions to
make the garbage collection more explicit.

Co-authored-by: Irit Katriel <[email protected]>

Lib/test/libregrtest/refleak.py

@@ -85,13 +85,15 @@ def get_pooled_int(value):
               flush=True)
 
     dash_R_cleanup(fs, ps, pic, zdc, abcs)
+    support.gc_collect()
 
     for i in rep_range:
         test_func()
+
         dash_R_cleanup(fs, ps, pic, zdc, abcs)
+        support.gc_collect()
 
-        # dash_R_cleanup() ends with collecting cyclic trash:
-        # read memory statistics immediately after.
+        # Read memory statistics immediately after the garbage collection
         alloc_after = getallocatedblocks() - _getquickenedcount()
         rc_after = gettotalrefcount()
         fd_after = fd_count()
@@ -166,9 +168,6 @@ def dash_R_cleanup(fs, ps, pic, zdc, abcs):
         zipimport._zip_directory_cache.clear()
         zipimport._zip_directory_cache.update(zdc)
 
-    # clear type cache
-    sys._clear_type_cache()
-
     # Clear ABC registries, restoring previously saved ABC registries.
     abs_classes = [getattr(collections.abc, a) for a in collections.abc.__all__]
     abs_classes = filter(isabstract, abs_classes)
@@ -179,8 +178,12 @@ def dash_R_cleanup(fs, ps, pic, zdc, abcs):
                     obj.register(ref())
             obj._abc_caches_clear()
 
+    # Clear caches
     clear_caches()
 
+    # Clear type cache at the end: previous function calls can modify types
+    sys._clear_type_cache()
+
 
 def warm_caches():
     # char cache

Lib/test/libregrtest/runtest.py

@@ -297,9 +297,13 @@ def _runtest_inner2(ns: Namespace, test_name: str) -> bool:
                 test_runner()
                 refleak = False
     finally:
-        cleanup_test_droppings(test_name, ns.verbose)
+        # First kill any dangling references to open files etc.
+        # This can also issue some ResourceWarnings which would otherwise get
+        # triggered during the following test run, and possibly produce
+        # failures.
+        support.gc_collect()
 
-    support.gc_collect()
+        cleanup_test_droppings(test_name, ns.verbose)
 
     if gc.garbage:
         support.environment_altered = True
@@ -330,6 +334,7 @@ def _runtest_inner(
 
     try:
         clear_caches()
+        support.gc_collect()
 
         with save_env(ns, test_name):
             refleak = _runtest_inner2(ns, test_name)
@@ -373,11 +378,6 @@ def _runtest_inner(
 
 
 def cleanup_test_droppings(test_name: str, verbose: int) -> None:
-    # First kill any dangling references to open files etc.
-    # This can also issue some ResourceWarnings which would otherwise get
-    # triggered during the following test run, and possibly produce failures.
-    support.gc_collect()
-
     # Try to clean up junk commonly left behind.  While tests shouldn't leave
     # any files or directories behind, when a test fails that can be tedious
     # for it to arrange.  The consequences can be especially nasty on Windows,

Lib/test/libregrtest/utils.py

@@ -217,5 +217,3 @@ def clear_caches():
     else:
         for f in typing._cleanups:
             f()
-
-    support.gc_collect()

Misc/NEWS.d/next/Tests/2021-09-01-17-17-44.bpo-44895.kV7H77.rst

@@ -0,0 +1,5 @@
+libregrtest now clears the type cache later to reduce the risk of false alarm
+when checking for reference leaks. Previously, the type cache was cleared too
+early and libregrtest raised a false alarm about reference leaks under very
+specific conditions.
+Patch by Irit Katriel and Victor Stinner.