You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
regrtest: change -R/--huntrleaks rule to decide if a test leaks references #74959
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee='https://github.com/vstinner'closed_at=<Date2018-09-19.23:10:15.484>created_at=<Date2017-06-26.21:49:23.239>labels= ['3.7', 'tests']
title='regrtest: change -R/--huntrleaks rule to decide if a test leaks references'updated_at=<Date2018-09-19.23:10:15.483>user='https://github.com/vstinner'
Currently, regrtest considers that a test file leaks if at least one run increased or decreased the global reference counter. The problem is that in practice, Python is full of singletons, caches, and other jokes of the garbage collector, etc.
To reduce false alarms and focus more on real bugs, I suggest to change the reference difference checker from:
I had to write this change to be able to bisect bpo-30775 reference leak. Without this change, my bisection tool picked the wrong tests and failed to find the tests which leaked. For example, it took a subtest of tests which was considered as leaking because the reference differences was [3, 0, 0]. This pattern more looks like a late initialization than a real leak. In the case of the bpo-30775, a leak was closer to [222, 222, 225]: much more references than just 3 :-) Many leaks for all runs :-)
It seems like the change works well since I didn't see any more false alarms on the Refleaks buildbots. I close the issue. The change has been applied to 2.7, 3.5, 3.6 and master branches.
changed the title [-]regrtest: change -R/--huntrleaks rule to decide if a test leaks[/-][+]regrtest: change -R/--huntrleaks rule to decide if a test leaks references[/+]on Jun 28, 2017
This change suddenly made detection of reference leaks a lot less effective. Now if you have a single 0 in the deltas list, it will consider there is no reference leak, even though all other values might be > 0.
Antoine Pitrou: "Victor, it would be nice if you could find a less brutal heuristic."
My current goal is to get green buildbots. If a buildbot is always red, it's hard to notify easily regressions.
Since I rejected "false alarms" from refleak, the refleak buildbot already helped to identify a reference leak regression. I am talking about the Windows Refleak buildbot:
I'm now working on the x86 Gentoo Refleaks 3.x which raised strange issues like bpo-31217.
I am already making regrtest stricter, but slowly, without breaking buildbots. It's a slow process. That's why I'm to "fix" dangling threads for example. By the way, I'm not done with dangling threads in master yet.
I'm happy if anyone is interested to help me to work on buildbots, which means in practice fixing a new different bug everyday :-) Slowly, the remaining bugs are the most complex ones.
The commit 6c2feab of bpo-31217 fixed a major random bug in the memory block check. So maybe we can start again to look to detect more bugs by replacing all() with any() again.
Activity
vstinner commentedon Jun 26, 2017
Currently, regrtest considers that a test file leaks if at least one run increased or decreased the global reference counter. The problem is that in practice, Python is full of singletons, caches, and other jokes of the garbage collector, etc.
To reduce false alarms and focus more on real bugs, I suggest to change the reference difference checker from:
to:
It would allow to ignore false positives like:
vstinner commentedon Jun 26, 2017
I had to write this change to be able to bisect bpo-30775 reference leak. Without this change, my bisection tool picked the wrong tests and failed to find the tests which leaked. For example, it took a subtest of tests which was considered as leaking because the reference differences was [3, 0, 0]. This pattern more looks like a late initialization than a real leak. In the case of the bpo-30775, a leak was closer to [222, 222, 225]: much more references than just 3 :-) Many leaks for all runs :-)
vstinner commentedon Jun 26, 2017
Real-world examples of false positives from the x86 Gentoo Refleaks 2.7 buildbot:
http://buildbot.python.org/all/builders/x86%20Gentoo%20Refleaks%202.7/builds/32/steps/test/logs/stdio
test_nntplib leaked [0, 85, 0] references, sum=85
test_multiprocessing leaked [0, 0, -35] references, sum=-35
vstinner commentedon Jun 27, 2017
New changeset 48b5c42 by Victor Stinner in branch 'master':
bpo-30776: reduce regrtest -R false positives (bpo-2422)
48b5c42
vstinner commentedon Jun 27, 2017
New changeset 35d2ca2 by Victor Stinner in branch '3.6':
[3.6] bpo-30523, bpo-30764, bpo-30776: Sync regrtest from master (bpo-2441)
35d2ca2
vstinner commentedon Jun 27, 2017
New changeset de1850b by Victor Stinner in branch '3.5':
[3.5] bpo-30523, bpo-30764, bpo-30776: Sync regrtest from master (bpo-2442)
de1850b
vstinner commentedon Jun 27, 2017
New changeset fea98bf by Victor Stinner in branch '2.7':
[2.7] bpo-30523, bpo-30764, bpo-30776: Sync regrtest from master (bpo-2444)
fea98bf
vstinner commentedon Jun 28, 2017
It seems like the change works well since I didn't see any more false alarms on the Refleaks buildbots. I close the issue. The change has been applied to 2.7, 3.5, 3.6 and master branches.
[-]regrtest: change -R/--huntrleaks rule to decide if a test leaks[/-][+]regrtest: change -R/--huntrleaks rule to decide if a test leaks references[/+]vstinner commentedon Jun 29, 2017
New changeset beeca6e by Victor Stinner in branch 'master':
bpo-30776: regrtest: reduce memleak false positive (bpo-2484)
beeca6e
vstinner commentedon Jun 30, 2017
New changeset a3ca94d by Victor Stinner in branch '3.6':
[3.6] bpo-29512, bpo-30776: Backport regrtest enhancements from master to 3.6 (bpo-2513)
a3ca94d
vstinner commentedon Jul 3, 2017
New changeset 0b12107 by Victor Stinner in branch '3.5':
[3.5] bpo-29512, bpo-30764, bpo-30776: Backport regrtest enhancements from 3.6 to 3.5 (bpo-2540)
0b12107
pitrou commentedon Aug 31, 2017
This change suddenly made detection of reference leaks a lot less effective. Now if you have a single 0 in the deltas list, it will consider there is no reference leak, even though all other values might be > 0.
See https://bugs.python.org/issue31317#msg301064 for an example.
Victor, it would be nice if you could find a less brutal heuristic.
vstinner commentedon Aug 31, 2017
Antoine Pitrou: "Victor, it would be nice if you could find a less brutal heuristic."
My current goal is to get green buildbots. If a buildbot is always red, it's hard to notify easily regressions.
Since I rejected "false alarms" from refleak, the refleak buildbot already helped to identify a reference leak regression. I am talking about the Windows Refleak buildbot:
http://buildbot.python.org/all/builders/AMD64%20Windows8.1%20Refleaks%203.x
I'm now working on the x86 Gentoo Refleaks 3.x which raised strange issues like bpo-31217.
I am already making regrtest stricter, but slowly, without breaking buildbots. It's a slow process. That's why I'm to "fix" dangling threads for example. By the way, I'm not done with dangling threads in master yet.
I'm happy if anyone is interested to help me to work on buildbots, which means in practice fixing a new different bug everyday :-) Slowly, the remaining bugs are the most complex ones.
pitrou commentedon Sep 1, 2017
Except that now it may hide actual regressions... False negatives are not better than false positives.
vstinner commentedon Sep 1, 2017
Please put this issue in its context. 6 months ago, there was no more Refleak buildbot and everything was broken.
If you want to make regrtest -R stricer, go ahead. But if a buildbot breaks, you will have to fix it.
I would prefer to first fix the Gentoo Refleak buildbot, since most previous builds failed.
vstinner commentedon Sep 1, 2017
The commit 6c2feab of bpo-31217 fixed a major random bug in the memory block check. So maybe we can start again to look to detect more bugs by replacing all() with any() again.