From 5aaac94eeb44697e92b0951385cd557bc27e0f6a Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 9 Apr 2019 14:23:47 +0200 Subject: [PATCH] bpo-36560: Fix reference leak hunting in regrtest (GH-12744) Fix reference leak hunting in regrtest: compute also deltas (of reference count, allocated memory blocks, 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() * Move calls to read memory statistics from dash_R_cleanup() to dash_R() * Pass regrtest 'ns' to dash_R() * 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(). --- Lib/test/libregrtest/refleak.py | 67 ++++++++++++------- Lib/test/libregrtest/runtest.py | 2 +- Lib/test/libregrtest/setup.py | 6 -- .../2019-04-09-14-08-02.bpo-36560._ejeOr.rst | 4 ++ 4 files changed, 46 insertions(+), 33 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2019-04-09-14-08-02.bpo-36560._ejeOr.rst diff --git a/Lib/test/libregrtest/refleak.py b/Lib/test/libregrtest/refleak.py index d68ea63b5b3..0bb8a0a2bf0 100644 --- a/Lib/test/libregrtest/refleak.py +++ b/Lib/test/libregrtest/refleak.py @@ -18,7 +18,7 @@ except ImportError: cls._abc_negative_cache, cls._abc_negative_cache_version) -def dash_R(the_module, test, indirect_test, huntrleaks): +def dash_R(ns, the_module, test_name, test_func): """Run a test multiple times, looking for reference leaks. Returns: @@ -32,6 +32,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 = copyreg.dispatch_table.copy() @@ -57,31 +61,50 @@ def dash_R(the_module, test, indirect_test, huntrleaks): def get_pooled_int(value): return int_pool.setdefault(value, value) - nwarmup, ntracked, fname = huntrleaks + nwarmup, ntracked, fname = ns.huntrleaks fname = os.path.join(support.SAVEDCWD, fname) repcount = nwarmup + ntracked + + # Pre-allocate to ensure that the loop doesn't allocate anything new + rep_range = list(range(repcount)) rc_deltas = [0] * repcount alloc_deltas = [0] * repcount fd_deltas = [0] * repcount + getallocatedblocks = sys.getallocatedblocks + gettotalrefcount = sys.gettotalrefcount + fd_count = support.fd_count - print("beginning", repcount, "repetitions", file=sys.stderr) - print(("1234567890"*(repcount//10 + 1))[:repcount], file=sys.stderr, - flush=True) # initialize variables to make pyflakes quiet rc_before = alloc_before = fd_before = 0 - for i in range(repcount): - indirect_test() - alloc_after, rc_after, fd_after = dash_R_cleanup(fs, ps, pic, zdc, - abcs) - print('.', end='', file=sys.stderr, flush=True) - if i >= nwarmup: - rc_deltas[i] = get_pooled_int(rc_after - rc_before) - alloc_deltas[i] = get_pooled_int(alloc_after - alloc_before) - fd_deltas[i] = get_pooled_int(fd_after - fd_before) + + if not ns.quiet: + print("beginning", repcount, "repetitions", file=sys.stderr) + print(("1234567890"*(repcount//10 + 1))[:repcount], file=sys.stderr, + flush=True) + + for i in rep_range: + test_func() + dash_R_cleanup(fs, ps, pic, zdc, abcs) + + # Collect cyclic trash and read memory statistics immediately after. + support.gc_collect() + alloc_after = getallocatedblocks() + rc_after = gettotalrefcount() + fd_after = fd_count() + + if not ns.quiet: + print('.', end='', file=sys.stderr, flush=True) + + rc_deltas[i] = get_pooled_int(rc_after - rc_before) + alloc_deltas[i] = get_pooled_int(alloc_after - alloc_before) + fd_deltas[i] = get_pooled_int(fd_after - fd_before) + alloc_before = alloc_after rc_before = rc_after fd_before = fd_after - print(file=sys.stderr) + + if not ns.quiet: + print(file=sys.stderr) # These checkers return False on success, True on failure def check_rc_deltas(deltas): @@ -112,7 +135,7 @@ def dash_R(the_module, test, indirect_test, huntrleaks): deltas = deltas[nwarmup:] if checker(deltas): msg = '%s leaked %s %s, sum=%s' % ( - test, deltas, item_name, sum(deltas)) + test_name, deltas, item_name, sum(deltas)) print(msg, file=sys.stderr, flush=True) with open(fname, "a") as refrep: print(msg, file=refrep) @@ -122,7 +145,7 @@ def dash_R(the_module, test, indirect_test, huntrleaks): def dash_R_cleanup(fs, ps, pic, zdc, abcs): - import gc, copyreg + import copyreg import collections.abc # Restore some original values. @@ -154,16 +177,8 @@ def dash_R_cleanup(fs, ps, pic, zdc, abcs): clear_caches() - # Collect cyclic trash and read memory statistics immediately after. - func1 = sys.getallocatedblocks - func2 = sys.gettotalrefcount - gc.collect() - return func1(), func2(), support.fd_count() - def clear_caches(): - import gc - # Clear the warnings registry, so they can be displayed again for mod in sys.modules.values(): if hasattr(mod, '__warningregistry__'): @@ -256,7 +271,7 @@ def clear_caches(): for f in typing._cleanups: f() - gc.collect() + support.gc_collect() def warm_caches(): diff --git a/Lib/test/libregrtest/runtest.py b/Lib/test/libregrtest/runtest.py index 4f218b769f9..0a9533c8a57 100644 --- a/Lib/test/libregrtest/runtest.py +++ b/Lib/test/libregrtest/runtest.py @@ -177,7 +177,7 @@ def runtest_inner(ns, test, display_failure=True): raise Exception("errors while loading tests") support.run_unittest(tests) if ns.huntrleaks: - refleak = dash_R(the_module, test, test_runner, ns.huntrleaks) + refleak = dash_R(ns, the_module, test, test_runner) else: test_runner() test_time = time.perf_counter() - start_time diff --git a/Lib/test/libregrtest/setup.py b/Lib/test/libregrtest/setup.py index 910aca1b1a6..9a6585af9d0 100644 --- a/Lib/test/libregrtest/setup.py +++ b/Lib/test/libregrtest/setup.py @@ -10,8 +10,6 @@ try: except ImportError: gc = None -from test.libregrtest.refleak import warm_caches - def setup_tests(ns): try: @@ -79,10 +77,6 @@ def setup_tests(ns): if ns.huntrleaks: unittest.BaseTestSuite._cleanup = False - # Avoid false positives due to various caches - # filling slowly with random data: - warm_caches() - if ns.memlimit is not None: support.set_memlimit(ns.memlimit) 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 new file mode 100644 index 00000000000..ad0f681ae87 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2019-04-09-14-08-02.bpo-36560._ejeOr.rst @@ -0,0 +1,4 @@ +Fix reference leak hunting in regrtest: compute also deltas (of reference +count, allocated memory blocks, file descriptor count) during warmup, to +ensure that everything is initialized before starting to hunt reference +leaks.