[Python-checkins] bpo-44895: libregrtest: refleak check clears types later (GH-28113)

[vstinner]

https://github.com/python/cpython/commit/679cb4781ea370c3b3ce40d3334dc404d7e9d92b
commit: 679cb4781ea370c3b3ce40d3334dc404d7e9d92b
branch: main
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2021-09-01T17:45:27+02:00
summary:

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 <1055913+iritkatriel at users.noreply.github.com>

files:
A Misc/NEWS.d/next/Tests/2021-09-01-17-17-44.bpo-44895.kV7H77.rst
M Lib/test/libregrtest/refleak.py
M Lib/test/libregrtest/runtest.py
M Lib/test/libregrtest/utils.py

diff --git a/Lib/test/libregrtest/refleak.py b/Lib/test/libregrtest/refleak.py
index b94826a5daf92c..096b5381cd9339 100644
--- a/Lib/test/libregrtest/refleak.py
+++ b/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
diff --git a/Lib/test/libregrtest/runtest.py b/Lib/test/libregrtest/runtest.py
index 489ab986cb4e5e..fe4693bad9ca6d 100644
--- a/Lib/test/libregrtest/runtest.py
+++ b/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,
diff --git a/Lib/test/libregrtest/utils.py b/Lib/test/libregrtest/utils.py
index 89d7e7e5354054..ad18b50f5db99d 100644
--- a/Lib/test/libregrtest/utils.py
+++ b/Lib/test/libregrtest/utils.py
@@ -217,5 +217,3 @@ def clear_caches():
     else:
         for f in typing._cleanups:
             f()
-
-    support.gc_collect()
diff --git a/Misc/NEWS.d/next/Tests/2021-09-01-17-17-44.bpo-44895.kV7H77.rst b/Misc/NEWS.d/next/Tests/2021-09-01-17-17-44.bpo-44895.kV7H77.rst
new file mode 100644
index 00000000000000..038466f8d6a4f3
--- /dev/null
+++ b/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.