bpo-36560: Fix reference leak hunting in regrtest (GH-12744) (GH-12745)

vstinner · web-flow · commit 9c14061a2c2d · 2019-04-09T18:01:17.000+02:00
Fix reference leak hunting in regrtest: compute also deltas (of reference count and file descriptor count) during warmup, to ensure that everything is initialized before starting to hunt reference leaks. Other changes: * Replace gc.collect() with support.gc_collect() in clear_caches() * dash_R() is now more quiet with --quiet option (don't display progress). * Precompute the full range for "for it in range(repcount):" to ensure that the iteration doesn't allocate anything new. * dash_R() now is responsible to call warm_caches(). (cherry picked from commit 5aaac94)
diff --git a/Lib/test/regrtest.py b/Lib/test/regrtest.py
@@ -529,8 +529,6 @@ def main(tests=None, testdir=None, verbose=0, quiet=False,
 
     if slaveargs is not None:
         args, kwargs = json.loads(slaveargs)
-        if kwargs['huntrleaks']:
-            warm_caches()
         if testdir:
             kwargs['testdir'] = testdir
         try:
@@ -541,9 +539,6 @@ def main(tests=None, testdir=None, verbose=0, quiet=False,
         print json.dumps(result)
         sys.exit(0)
 
-    if huntrleaks:
-        warm_caches()
-
     good = []
     bad = []
     skipped = []
@@ -1332,7 +1327,7 @@ def runtest_inner(test, verbose, quiet, huntrleaks=False, pgo=False, testdir=Non
                 indirect_test = getattr(the_module, "test_main", None)
                 if huntrleaks:
                     refleak = dash_R(the_module, test, indirect_test,
-                        huntrleaks)
+                        huntrleaks, quiet)
                 else:
                     if indirect_test is not None:
                         indirect_test()
@@ -1425,7 +1420,7 @@ def cleanup_test_droppings(testname, verbose):
             print >> sys.stderr, ("%r left behind %s %r and it couldn't be "
                 "removed: %s" % (testname, kind, name, msg))
 
-def dash_R(the_module, test, indirect_test, huntrleaks):
+def dash_R(the_module, test, indirect_test, huntrleaks, quiet):
     """Run a test multiple times, looking for reference leaks.
 
     Returns:
@@ -1438,6 +1433,10 @@ def dash_R(the_module, test, indirect_test, huntrleaks):
         raise Exception("Tracking reference leaks requires a debug build "
                         "of Python")
 
+    # Avoid false positives due to various caches
+    # filling slowly with random data:
+    warm_caches()
+
     # Save current values for dash_R_cleanup() to restore.
     fs = warnings.filters[:]
     ps = copy_reg.dispatch_table.copy()
@@ -1457,6 +1456,14 @@ def dash_R(the_module, test, indirect_test, huntrleaks):
         for obj in abc.__subclasses__() + [abc]:
             abcs[obj] = obj._abc_registry.copy()
 
+    # bpo-31217: Integer pool to get a single integer object for the same
+    # value. The pool is used to prevent false alarm when checking for memory
+    # block leaks. Fill the pool with values in -1000..1000 which are the most
+    # common (reference, memory block, file descriptor) differences.
+    int_pool = {value: value for value in range(-1000, 1000)}
+    def get_pooled_int(value):
+        return int_pool.setdefault(value, value)
+
     if indirect_test:
         def run_the_test():
             indirect_test()
@@ -1467,27 +1474,39 @@ def run_the_test():
     deltas = []
     nwarmup, ntracked, fname = huntrleaks
     fname = os.path.join(support.SAVEDCWD, fname)
+
+    # Pre-allocate to ensure that the loop doesn't allocate anything new
     repcount = nwarmup + ntracked
-    rc_deltas = [0] * ntracked
-    fd_deltas = [0] * ntracked
+    rc_deltas = [0] * repcount
+    fd_deltas = [0] * repcount
+    rep_range = list(range(repcount))
+
+    if not quiet:
+        print >> sys.stderr, "beginning", repcount, "repetitions"
+        print >> sys.stderr, ("1234567890"*(repcount//10 + 1))[:repcount]
 
-    print >> sys.stderr, "beginning", repcount, "repetitions"
-    print >> sys.stderr, ("1234567890"*(repcount//10 + 1))[:repcount]
     dash_R_cleanup(fs, ps, pic, zdc, abcs)
+
     # initialize variables to make pyflakes quiet
     rc_before = fd_before = 0
-    for i in range(repcount):
+
+    for i in rep_range:
         run_the_test()
-        sys.stderr.write('.')
+
+        if not quiet:
+            sys.stderr.write('.')
+
         dash_R_cleanup(fs, ps, pic, zdc, abcs)
+
         rc_after = sys.gettotalrefcount()
         fd_after = support.fd_count()
-        if i >= nwarmup:
-            rc_deltas[i - nwarmup] = rc_after - rc_before
-            fd_deltas[i - nwarmup] = fd_after - fd_before
+        rc_deltas[i] = get_pooled_int(rc_after - rc_before)
+        fd_deltas[i] = get_pooled_int(fd_after - fd_before)
         rc_before = rc_after
         fd_before = fd_after
-    print >> sys.stderr
+
+    if not quiet:
+        print >> sys.stderr
 
     # These checkers return False on success, True on failure
     def check_rc_deltas(deltas):
@@ -1513,6 +1532,7 @@ def check_fd_deltas(deltas):
         (rc_deltas, 'references', check_rc_deltas),
         (fd_deltas, 'file descriptors', check_fd_deltas)
     ]:
+        deltas = deltas[nwarmup:]
         if checker(deltas):
             msg = '%s leaked %s %s, sum=%s' % (test, deltas, item_name, sum(deltas))
             print >> sys.stderr, msg
@@ -1647,7 +1667,7 @@ def clear_caches():
         ctypes._reset_cache()
 
     # Collect cyclic trash.
-    gc.collect()
+    support.gc_collect()
 
 def warm_caches():
     """Create explicitly internal singletons which are created on demand
diff --git a/Misc/NEWS.d/next/Tests/2019-04-09-14-08-02.bpo-36560._ejeOr.rst b/Misc/NEWS.d/next/Tests/2019-04-09-14-08-02.bpo-36560._ejeOr.rst
@@ -0,0 +1,3 @@
+Fix reference leak hunting in regrtest: compute also deltas (of reference count
+and file descriptor count) during warmup, to ensure that everything is
+initialized before starting to hunt reference leaks.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+Fix reference leak hunting in regrtest: compute also deltas (of reference count`
	`2`	`+and file descriptor count) during warmup, to ensure that everything is`
	`3`	`+initialized before starting to hunt reference leaks.`