classification
Title: test_idle: idlelib.configdialog leaks references
Type: resource usage Stage: resolved
Components: IDLE, Tests Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: terry.reedy, vstinner
Priority: normal Keywords:

Created on 2017-08-07 09:24 by vstinner, last changed 2017-08-10 22:58 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
findleak.py terry.reedy, 2017-08-07 18:09
Pull Requests
URL Status Linked Edit
PR 3014 closed vstinner, 2017-08-07 10:20
PR 3016 merged terry.reedy, 2017-08-07 16:18
PR 3018 merged terry.reedy, 2017-08-07 18:25
Messages (11)
msg299834 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-07 09:24
Starting at the following build, test_idle started to leak references:
http://buildbot.python.org/all/builders/AMD64%20Windows8.1%20Refleaks%203.x/builds/60/steps/test/logs/stdio

Example of leak:

$ ./python -m test -R 3:3 -u all test_idle -m idlelib.idle_test.test_configdialog.FontPageTest.test_set_samples
(...)
test_idle leaked [2168, 2168, 2168] references, sum=6504
test_idle leaked [1152, 1154, 1155] memory blocks, sum=3461
(...)

A bisection identified this commit:

commit 9397e2a87ed6e0e724ad71a0c751553620cb775e
Author: csabella <cheryl.sabella@gmail.com>
Date:   Sun Jul 30 13:34:25 2017 -0400

    bpo-31004: IDLE: Factor out FontPage class from configdialog (step 1) (#2905)
    
    The slightly modified tests continue to pass. The General test
    broken by the switch to Notebook is fixed.
    Patch mostly by Cheryl Sabella.
msg299837 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-07 09:54
I don't know anything about idlelib, but it seems like the following patch fixes the failing test:

diff --git a/Lib/idlelib/idle_test/test_configdialog.py b/Lib/idlelib/idle_test/test_configdialog.py
index aff3c2f..9f495fd 100644
--- a/Lib/idlelib/idle_test/test_configdialog.py
+++ b/Lib/idlelib/idle_test/test_configdialog.py
@@ -41,6 +41,7 @@ def tearDownModule():
     global root, dialog
     idleConf.userCfg = usercfg
     tracers.detach()
+    tracers.clear()
     del dialog
     root.update_idletasks()
     root.destroy()

Extract of ConfigDialog.create_page_highlight():

        self.builtin_theme = tracers.add(
                StringVar(parent), self.var_changed_builtin_theme)

This method contains many tracers.add() calls, but tearDownModule() of Lib/idlelib/idle_test/test_configdialog.py only calls "tracers.detach()" and detatch() doesn't clear VarTrace.untraced list.
msg299857 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-08-07 18:09
Attached is an improved idlelib.idle_test.test_* leak checker that uses the --match option.
msg299858 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-08-07 18:22
New changeset 733d0f63c562090a2b840859b96028d6ec0a4803 by Terry Jan Reedy in branch 'master':
bpo-31130: IDLE -- stop leaks in test_configdialog. (#3016)
https://github.com/python/cpython/commit/733d0f63c562090a2b840859b96028d6ec0a4803
msg299862 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-08-07 19:20
New changeset 9d7d928b5853b921ed27f1e535dfe8174169854c by Terry Jan Reedy in branch '3.6':
[3.6] bpo-31130: IDLE -- stop leaks in test_configdialog. (GH-3016) (#3018)
https://github.com/python/cpython/commit/9d7d928b5853b921ed27f1e535dfe8174169854c
msg299864 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-07 20:12
I confirm that test_idle doesn't leak anymore:

$ ./python -m test -R 3:3 -u all test_idle 
1 test OK.
msg299865 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-07 20:15
The merged change is the correct fix. My change was only a workaround.

Thanks for the fix Terry ;-)
msg300115 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-08-10 20:57
Victor: Something seems to have changed in the last few days.  When I merged, both specific classes
python -m test -R 3:3 -u gui -v test_idle -m idlelib.idle_test.test_configdialog.FontPageTest.*
and the module as a whole passed.
python -m test -R 3:3 -u gui -v test_idle -m idlelib.idle_test.test_configdialog.*.*

Today, not having touched configdialog or test_configdialog, test_configdialog as a whole passes but FontPageTest, IndentTest, and GenPageTest fail with 
test_idle leaked [1, 1, 1, 1] memory blocks, sum=4
(Tested individually, VarTraceTest and the empty HighlightTest pass.)
I presume that cleanUpModule still runs with the restriction.

Could any of your other recent changes to regrtest have introduced a spurious 1 block error when tests are filtered?
msg300116 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-10 21:28
> test_idle leaked [1, 1, 1, 1] memory blocks, sum=4

While reference leaks are more or less stable, the check on memory blocks is fragile.

I'm unable to reproduce your issue on Linux. I tested "./python -m test -R 3:3 -u gui -v test_idle". If you have a reproductible failure, please open a new issue since this one is closed.

If you consider that it's a recent regression, you can go back in history using git checkout <old sha1>, or even git bisect.

If you open a new issue, you may want to use "python -m test.bisect ..." to identify which test leaks memory blocks.
msg300125 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-08-10 22:00
My message was most a heads-up.  As I said, I only get the 1 block leak with a micro test of a class or method.  It is possible that even that is Windows-specific.  As long as you only care that test_idle not leak, I don't care about this either.
msg300126 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-10 22:58
> As I said, I only get the 1 block leak with a micro test of a class or method.

Yeah, again, the memory block check is fragile. It might be an legit internal cache filled for good reasons. It can be a free list. It can be a lot of things. It's really hard to track such very tiny memory allocation. "leaked [1, 1, 1, 1] memory blocks" can be a false alarm, it's not big.
History
Date User Action Args
2017-08-10 22:58:52vstinnersetmessages: + msg300126
2017-08-10 22:00:39terry.reedysetmessages: + msg300125
2017-08-10 21:28:13vstinnersetmessages: + msg300116
2017-08-10 20:57:45terry.reedysetmessages: + msg300115
2017-08-07 20:15:47vstinnersetmessages: + msg299865
2017-08-07 20:12:38vstinnersetmessages: + msg299864
2017-08-07 19:20:36terry.reedysetstatus: open -> closed
resolution: fixed
stage: resolved
2017-08-07 19:20:08terry.reedysetmessages: + msg299862
2017-08-07 18:25:03terry.reedysetpull_requests: + pull_request3051
2017-08-07 18:22:46terry.reedysetmessages: + msg299858
2017-08-07 18:09:43terry.reedysetfiles: + findleak.py

nosy: + terry.reedy
messages: + msg299857

assignee: terry.reedy
components: + IDLE, Tests
2017-08-07 16:18:05terry.reedysetpull_requests: + pull_request3049
2017-08-07 10:20:07vstinnersetpull_requests: + pull_request3047
2017-08-07 09:54:21vstinnersetmessages: + msg299837
2017-08-07 09:28:04vstinnersetversions: + Python 3.6
2017-08-07 09:24:47vstinnercreate