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: regrtest: change -R/--huntrleaks rule to decide if a test leaks references
Type: Stage: resolved
Components: Tests Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: vstinner Nosy List: pitrou, serhiy.storchaka, vstinner
Priority: normal Keywords:

Created on 2017-06-26 21:49 by vstinner, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 2422 merged vstinner, 2017-06-26 21:51
PR 2441 merged vstinner, 2017-06-27 13:50
PR 2442 merged vstinner, 2017-06-27 14:15
PR 2444 merged vstinner, 2017-06-27 14:49
PR 2484 merged vstinner, 2017-06-29 07:42
PR 2513 merged vstinner, 2017-06-30 15:11
PR 2540 merged vstinner, 2017-07-03 10:57
Messages (16)
msg296950 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-26 21:49
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:

     def check_rc_deltas(deltas):
        return any(deltas)

to:

     def check_rc_deltas(deltas):
        return all(delta>=1 for delta in deltas)

It would allow to ignore false positives like:

* [3, 0, 0]
* [0, 1, 0]
* [8, -8, 1]
msg296952 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-26 21:53
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 :-)
msg296977 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-26 23:51
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
msg296978 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-27 00:02
New changeset 48b5c422ffb03affb00c184b9a99e5537be92732 by Victor Stinner in branch 'master':
bpo-30776: reduce regrtest -R false positives (#2422)
https://github.com/python/cpython/commit/48b5c422ffb03affb00c184b9a99e5537be92732
msg297034 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-27 14:04
New changeset 35d2ca2b94a6ff29e763ddb7727166f0592edfa2 by Victor Stinner in branch '3.6':
[3.6] bpo-30523, bpo-30764, bpo-30776: Sync regrtest from master (#2441)
https://github.com/python/cpython/commit/35d2ca2b94a6ff29e763ddb7727166f0592edfa2
msg297041 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-27 14:35
New changeset de1850bb03f8225cbff85f437b6e972bf9b68c2a by Victor Stinner in branch '3.5':
[3.5] bpo-30523, bpo-30764, bpo-30776: Sync regrtest from master (#2442)
https://github.com/python/cpython/commit/de1850bb03f8225cbff85f437b6e972bf9b68c2a
msg297044 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-27 14:56
New changeset fea98bfcff6ccf9142daa97677fe86c1fdf8e63e by Victor Stinner in branch '2.7':
[2.7] bpo-30523, bpo-30764, bpo-30776: Sync regrtest from master (#2444)
https://github.com/python/cpython/commit/fea98bfcff6ccf9142daa97677fe86c1fdf8e63e
msg297085 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-28 00:41
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.
msg297262 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-29 08:32
New changeset beeca6e1e5fd01531b1db7059498b13d07dca525 by Victor Stinner in branch 'master':
bpo-30776: regrtest: reduce memleak false positive (#2484)
https://github.com/python/cpython/commit/beeca6e1e5fd01531b1db7059498b13d07dca525
msg297420 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-06-30 15:31
New changeset a3ca94d0504157a112a1f89bfe8be1307116fc73 by Victor Stinner in branch '3.6':
[3.6] bpo-29512, bpo-30776: Backport regrtest enhancements from master to 3.6 (#2513)
https://github.com/python/cpython/commit/a3ca94d0504157a112a1f89bfe8be1307116fc73
msg297565 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-07-03 11:17
New changeset 0b1210739d12c26e4a161ffd494bd572d49b2483 by Victor Stinner in branch '3.5':
[3.5] bpo-29512, bpo-30764, bpo-30776: Backport regrtest enhancements from 3.6 to 3.5 (#2540)
https://github.com/python/cpython/commit/0b1210739d12c26e4a161ffd494bd572d49b2483
msg301066 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-08-31 22:08
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.
msg301071 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-31 22:38
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.
msg301078 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-09-01 00:18
> Since I rejected "false alarms" from refleak, the refleak buildbot already helped to identify a reference leak regression.

Except that now it may hide actual regressions... False negatives are not better than false positives.
msg301080 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-01 00:35
> Except that now it may hide actual regressions... False negatives are not better than false positives.

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.
msg301102 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-01 13:18
The commit 6c2feabc5dac2f3049b15134669e9ad5af573193 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.
History
Date User Action Args
2022-04-11 14:58:48adminsetgithub: 74959
2018-09-19 23:10:15vstinnersetstatus: open -> closed
resolution: fixed
stage: needs patch -> resolved
2017-09-01 13:18:22vstinnersetmessages: + msg301102
2017-09-01 00:35:42vstinnersetmessages: + msg301080
2017-09-01 00:18:30pitrousetmessages: + msg301078
2017-08-31 22:38:45vstinnersetmessages: + msg301071
2017-08-31 22:08:30pitrousetstatus: closed -> open

assignee: vstinner

nosy: + serhiy.storchaka, pitrou
messages: + msg301066
resolution: fixed -> (no value)
stage: resolved -> needs patch
2017-07-03 11:17:00vstinnersetmessages: + msg297565
2017-07-03 10:57:21vstinnersetpull_requests: + pull_request2608
2017-06-30 15:31:19vstinnersetmessages: + msg297420
2017-06-30 15:11:40vstinnersetpull_requests: + pull_request2583
2017-06-29 08:32:51vstinnersetmessages: + msg297262
2017-06-29 07:42:43vstinnersetpull_requests: + pull_request2543
2017-06-28 00:41:38vstinnersetstatus: open -> closed
title: regrtest: change -R/--huntrleaks rule to decide if a test leaks -> regrtest: change -R/--huntrleaks rule to decide if a test leaks references
messages: + msg297085

resolution: fixed
stage: resolved
2017-06-27 14:56:45vstinnersetmessages: + msg297044
2017-06-27 14:49:30vstinnersetpull_requests: + pull_request2500
2017-06-27 14:35:20vstinnersetmessages: + msg297041
2017-06-27 14:15:35vstinnersetpull_requests: + pull_request2496
2017-06-27 14:04:18vstinnersetmessages: + msg297034
2017-06-27 13:50:38vstinnersetpull_requests: + pull_request2493
2017-06-27 00:02:07vstinnersetmessages: + msg296978
2017-06-26 23:51:39vstinnersetmessages: + msg296977
2017-06-26 21:53:20vstinnersetmessages: + msg296952
2017-06-26 21:51:13vstinnersetpull_requests: + pull_request2472
2017-06-26 21:49:23vstinnercreate