This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: False positives when running leak tests with -R 1:1
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.8, Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: pablogsal, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2018-06-15 22:42 by pablogsal, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 7735 merged pablogsal, 2018-06-15 22:59
PR 7736 merged pablogsal, 2018-06-15 23:08
PR 7933 closed miss-islington, 2018-06-26 21:07
PR 7934 closed miss-islington, 2018-06-26 21:07
PR 7935 merged vstinner, 2018-06-26 21:11
PR 7936 merged vstinner, 2018-06-26 21:31
PR 7937 merged vstinner, 2018-06-26 21:52
Messages (15)
msg319688 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2018-06-15 22:42
I am not sure is a problem we can do something about but right know if you run the refleak tests with low repetitions it reports leaks:

./python -m test test_list -R 1:1
Run tests sequentially
0:00:00 load avg: 0.66 [1/1] test_list
beginning 2 repetitions
12
..

test_list leaked [3] memory blocks, sum=3
test_list failed

== Tests result: FAILURE ==

1 test failed:
    test_list

Total duration: 1 sec 759 ms
Tests result: FAILURE

This also happens with other low numbers: 

./python -m test test_list -R 1:2

Obviously using this numbers is "wrong" (there is not enough repetitions to get meaningful results). The only problem I see is that if you are not aware of this limitation (in the case this is a real limitation on how `dash_R` works) the output is a bit misleading. 

Should we leave this as it is or try to improve the output?
msg319689 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-15 22:51
> test_list leaked [3] memory blocks, sum=3

Memory block leaks are very different from reference leaks. Memory block are low level allocations. Python has *many* internal caches: tuple uses an internal "free list" for example. The first runs of the tests (2 runs when using -R 2:3) is used to warmup these caches.

Maybe regrtest -R should raise an error, or at least emit a big warning when using -R with less than 3 warmup runs.

By the way, regrtest has a very old bug: -R 3:3 runs the test 7 times, not 6 times. See runtest_inner() in Lib/test/libregrtest/runtest.py:

            test_runner()
            if ns.huntrleaks:
                refleak = dash_R(the_module, test, test_runner, ns.huntrleaks)

The code should be:

            if ns.huntrleaks:
                refleak = dash_R(the_module, test, test_runner, ns.huntrleaks)
            else:
                test_runner()

Do you want to write a PR for that? I should make our Refleaks buildbots 1/7 faster ;-)
msg319690 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2018-06-15 22:56
Let's make the buildbots happier!
msg319691 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-15 23:17
I tested PR 7735:

vstinner@apu$ ./python -m test -R 0:0 test_os -m test_access
Run tests sequentially
0:00:00 load avg: 0.19 [1/1] test_os
beginning 0 repetitions


test_os leaked [] references, sum=0
test_os leaked [] memory blocks, sum=0
test_os failed

== Tests result: FAILURE ==

1 test failed:
    test_os

Total duration: 63 ms
Tests result: FAILURE

vstinner@apu$ ./python -m test -R 0:1 test_os -m test_access
Run tests sequentially
0:00:00 load avg: 0.35 [1/1] test_os
beginning 1 repetitions
1
.
test_os leaked [280435] references, sum=280435
test_os leaked [91518] memory blocks, sum=91518
test_os leaked [4] file descriptors, sum=4
test_os failed

== Tests result: FAILURE ==

1 test failed:
    test_os

Total duration: 95 ms
Tests result: FAILURE

vstinner@apu$ ./python -m test -R 1:0 test_os -m test_access
Run tests sequentially
0:00:00 load avg: 0.16 [1/1] test_os
beginning 1 repetitions
1
.
test_os leaked [] references, sum=0
test_os leaked [] memory blocks, sum=0
test_os failed

== Tests result: FAILURE ==

1 test failed:
    test_os

Total duration: 95 ms
Tests result: FAILURE



Hum, we should require at least one run and at least one warmup: -R 1:1 should be the bare minimum.

By the way, it seems like negative numbers are currently accepted, whereas it doesn't make sense:

vstinner@apu$ ./python -m test -R 0:-2 test_list
Run tests sequentially
0:00:00 load avg: 0.31 [1/1] test_list
beginning -2 repetitions
(...)

It would fix this bug as well.
msg319693 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2018-06-15 23:44
Updated PR7735 with the checks for invalid parameters.
msg319694 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-16 00:20
New changeset cac4fef8860e66a9da67d09762f5b614b9471a12 by Victor Stinner (Pablo Galindo) in branch 'master':
bpo-33873: regrtest: Add warning on -R 1:3 (GH-7736)
https://github.com/python/cpython/commit/cac4fef8860e66a9da67d09762f5b614b9471a12
msg320077 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-20 13:52
Once PR 7735 will be merged, it may be worth it to backport the enhancements to other branches. See maybe bpo-33718 to check if there are other changes that should be backportd. It can help to backport the change to 2.7 (which is very different).
msg320118 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2018-06-20 21:31
I think commit4ffe9c2b251f6e027b26250b7a2618e78d4edd22 from bpo-33718 should be backported IMO:

bpo-33718: regrtest: use format_duration() to display failed tests (GH-7686)
https://github.com/python/cpython/commit/4ffe9c2b251f6e027b26250b7a2618e78d4edd22
msg320120 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-20 21:35
> I think commit 4ffe9c2b251f6e027b26250b7a2618e78d4edd22 from bpo-33718 should be backported IMO: ...

Once PR 7735 will be merged, we can backport recent regrtest commits at once into 3.7, then backport the 3.7 change to 3.6 and 2.7.
msg320492 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2018-06-26 14:17
New changeset 58ed7307ea0b5c5aa052291ebc3030f314f938d8 by Pablo Galindo in branch 'master':
bpo-33873: Fix bug in `runtest.py` and add checks for invalid `-R` parameters (GH-7735)
https://github.com/python/cpython/commit/58ed7307ea0b5c5aa052291ebc3030f314f938d8
msg320510 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-26 21:14
Since I didn't backport yet the commit 4ffe9c2b251f6e027b26250b7a2618e78d4edd22 (format duration) from master to other branches, I will backport the commit 58ed7307ea0b5c5aa052291ebc3030f314f938d8 (-R fix) manually to other branches with other recent regrtest changes.
msg320514 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-26 21:47
New changeset d1f9481b7a2d31c40fca1347ef99d819eb656ce7 by Victor Stinner in branch '3.7':
bpo-33873: Backport regrtest from master to 3.7 (GH-7935)
https://github.com/python/cpython/commit/d1f9481b7a2d31c40fca1347ef99d819eb656ce7
msg320517 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-26 21:57
New changeset 5430c14aba319f83b18879575244ba429e8c1d81 by Victor Stinner in branch '2.7':
[2.7] bpo-33873: Backport regrtest from master (GH-7936)
https://github.com/python/cpython/commit/5430c14aba319f83b18879575244ba429e8c1d81
msg320522 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-26 22:44
New changeset 1d55888a78bcefed3723fb7e48df2a75b4f0adb0 by Victor Stinner in branch '3.6':
bpo-33873: Backport regrtest from master to 3.7 (GH-7935) (GH-7937)
https://github.com/python/cpython/commit/1d55888a78bcefed3723fb7e48df2a75b4f0adb0
msg320524 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-06-26 22:45
Pablo Galindo Salgado fixed the bug in master, I backported his fix to 2.7, 3.6 and 3.7 branches. Thanks Pablo!
History
Date User Action Args
2022-04-11 14:59:01adminsetgithub: 78054
2018-06-26 22:45:32vstinnersetstatus: open -> closed
versions: + Python 2.7, Python 3.6, Python 3.7
messages: + msg320524

resolution: fixed
stage: patch review -> resolved
2018-06-26 22:44:51vstinnersetmessages: + msg320522
2018-06-26 21:57:15vstinnersetmessages: + msg320517
2018-06-26 21:52:24vstinnersetpull_requests: + pull_request7546
2018-06-26 21:47:39vstinnersetmessages: + msg320514
2018-06-26 21:31:39vstinnersetpull_requests: + pull_request7544
2018-06-26 21:14:15vstinnersetmessages: + msg320510
2018-06-26 21:11:59vstinnersetpull_requests: + pull_request7542
2018-06-26 21:07:53miss-islingtonsetpull_requests: + pull_request7541
2018-06-26 21:07:02miss-islingtonsetpull_requests: + pull_request7540
2018-06-26 14:17:36pablogsalsetmessages: + msg320492
2018-06-20 21:35:03vstinnersetmessages: + msg320120
2018-06-20 21:31:29pablogsalsetmessages: + msg320118
2018-06-20 13:52:04vstinnersetmessages: + msg320077
2018-06-16 00:21:00vstinnersetmessages: + msg319694
2018-06-15 23:44:31pablogsalsetmessages: + msg319693
2018-06-15 23:17:47vstinnersetmessages: + msg319691
2018-06-15 23:08:52pablogsalsetpull_requests: + pull_request7347
2018-06-15 22:59:38pablogsalsetkeywords: + patch
stage: patch review
pull_requests: + pull_request7346
2018-06-15 22:56:27pablogsalsetmessages: + msg319690
2018-06-15 22:52:56pablogsalsettitle: False positives when running refleaks tests with -R 1:1 -> False positives when running leak tests with -R 1:1
2018-06-15 22:51:44vstinnersetmessages: + msg319689
2018-06-15 22:42:06pablogsalcreate