Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_idle: idlelib.configdialog leaks references #75313

Closed
vstinner opened this issue Aug 7, 2017 · 11 comments
Closed

test_idle: idlelib.configdialog leaks references #75313

vstinner opened this issue Aug 7, 2017 · 11 comments
Assignees
Labels
3.7 (EOL) end of life performance Performance or resource usage tests Tests in the Lib/test dir topic-IDLE

Comments

@vstinner
Copy link
Member

vstinner commented Aug 7, 2017

BPO 31130
Nosy @terryjreedy, @vstinner
PRs
  • bpo-31130: Fix test_idle reference leaks #3014
  • bpo-31130: IDLE -- stop leak in test_configdialog. #3016
  • [3.6] bpo-31130: IDLE -- stop leaks in test_configdialog. (GH-3016) #3018
  • Files
  • findleak.py
  • 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/terryjreedy'
    closed_at = <Date 2017-08-07.19:20:36.549>
    created_at = <Date 2017-08-07.09:24:47.014>
    labels = ['expert-IDLE', '3.7', 'tests', 'performance']
    title = 'test_idle: idlelib.configdialog leaks references'
    updated_at = <Date 2017-08-10.22:58:52.081>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2017-08-10.22:58:52.081>
    actor = 'vstinner'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2017-08-07.19:20:36.549>
    closer = 'terry.reedy'
    components = ['IDLE', 'Tests']
    creation = <Date 2017-08-07.09:24:47.014>
    creator = 'vstinner'
    dependencies = []
    files = ['47059']
    hgrepos = []
    issue_num = 31130
    keywords = []
    message_count = 11.0
    messages = ['299834', '299837', '299857', '299858', '299862', '299864', '299865', '300115', '300116', '300125', '300126']
    nosy_count = 2.0
    nosy_names = ['terry.reedy', 'vstinner']
    pr_nums = ['3014', '3016', '3018']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'resource usage'
    url = 'https://bugs.python.org/issue31130'
    versions = ['Python 3.6', 'Python 3.7']

    @vstinner
    Copy link
    Member Author

    vstinner commented Aug 7, 2017

    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 9397e2a
    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) (bpo-2905)
    
    The slightly modified tests continue to pass. The General test
    broken by the switch to Notebook is fixed.
    Patch mostly by Cheryl Sabella.
    

    @vstinner vstinner added 3.7 (EOL) end of life performance Performance or resource usage labels Aug 7, 2017
    @vstinner
    Copy link
    Member Author

    vstinner commented Aug 7, 2017

    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.

    @terryjreedy
    Copy link
    Member

    Attached is an improved idlelib.idle_test.test_* leak checker that uses the --match option.

    @terryjreedy terryjreedy added topic-IDLE tests Tests in the Lib/test dir labels Aug 7, 2017
    @terryjreedy terryjreedy self-assigned this Aug 7, 2017
    @terryjreedy
    Copy link
    Member

    New changeset 733d0f6 by Terry Jan Reedy in branch 'master':
    bpo-31130: IDLE -- stop leaks in test_configdialog. (bpo-3016)
    733d0f6

    @terryjreedy
    Copy link
    Member

    New changeset 9d7d928 by Terry Jan Reedy in branch '3.6':
    [3.6] bpo-31130: IDLE -- stop leaks in test_configdialog. (GH-3016) (bpo-3018)
    9d7d928

    @vstinner
    Copy link
    Member Author

    vstinner commented Aug 7, 2017

    I confirm that test_idle doesn't leak anymore:

    $ ./python -m test -R 3:3 -u all test_idle 
    1 test OK.

    @vstinner
    Copy link
    Member Author

    vstinner commented Aug 7, 2017

    The merged change is the correct fix. My change was only a workaround.

    Thanks for the fix Terry ;-)

    @terryjreedy
    Copy link
    Member

    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?

    @vstinner
    Copy link
    Member Author

    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.

    @terryjreedy
    Copy link
    Member

    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.

    @vstinner
    Copy link
    Member Author

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life performance Performance or resource usage tests Tests in the Lib/test dir topic-IDLE
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants