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 is leaking references #88147

Closed
pablogsal opened this issue Apr 29, 2021 · 13 comments
Closed

test_idle is leaking references #88147

pablogsal opened this issue Apr 29, 2021 · 13 comments
Assignees
Labels

Comments

@pablogsal
Copy link
Member

BPO 43981
Nosy @terryjreedy, @taleinat, @pablogsal
PRs
  • bpo-43981: Fix error in idle-test leak test #25739
  • bpo-43981: Fix reference leaks in test_squeezer #25758
  • 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 2021-04-30.18:34:35.785>
    created_at = <Date 2021-04-29.21:29:20.378>
    labels = ['expert-IDLE', '3.10', 'release-blocker']
    title = 'test_idle is leaking references'
    updated_at = <Date 2021-05-01.09:57:36.952>
    user = 'https://github.com/pablogsal'

    bugs.python.org fields:

    activity = <Date 2021-05-01.09:57:36.952>
    actor = 'taleinat'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2021-04-30.18:34:35.785>
    closer = 'pablogsal'
    components = ['IDLE']
    creation = <Date 2021-04-29.21:29:20.378>
    creator = 'pablogsal'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43981
    keywords = ['patch']
    message_count = 13.0
    messages = ['392353', '392354', '392376', '392377', '392381', '392424', '392467', '392474', '392475', '392494', '392495', '392512', '392579']
    nosy_count = 3.0
    nosy_names = ['terry.reedy', 'taleinat', 'pablogsal']
    pr_nums = ['25739', '25758']
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue43981'
    versions = ['Python 3.10']

    @pablogsal
    Copy link
    Member Author

    See for example:

    https://buildbot.python.org/all/#/builders/511/builds/10/steps/4/logs/stdio

    test_idle leaked [684, 684, 684] references, sum=2052
    test_idle leaked [282, 282, 282] memory blocks, sum=846

    @pablogsal
    Copy link
    Member Author

    Marking this as a release blocker

    @terryjreedy
    Copy link
    Member

    The only recently added tests are the 11 in test_sidebar.ShellSidebarTest. Leak testing just these with
    python -m test -R3:3 -ugui -m *Sidebar* test_idle
    does not fail. Leak test all of test_idle with
    >python -m test -R3:3 -v -ugui test_idle
    ends after the second round with two error failures.

    FAIL: test_menudefs (idlelib.idle_test.test_mainmenu.MainMenuTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "f:\dev\3x\lib\idlelib\idle_test\test_mainmenu.py", line 15, in test_menudefs
        self.assertEqual(actual, expect)
    AssertionError: Lists differ:
      ['application', 'file', 'edit', 'format', '[47 chars]elp'] !=
      ['file', 'edit', 'format', 'run', 'shell', [32 chars]elp']

    The 'application' menu was by added by a call to macosx.setupApp, which calls overrideRootMenu, in ShellSidebarTest.setUpClass. This call is not needed for this testcase, so the first PR replaces it with a comment referring to this issue.

    Note: when test_macosc calls setupApp, overrideRootMenu is replaced with a mock.

    The second execution of ShellSidebarTest.setUpClass failed with

    ERROR: setUpClass (idlelib.idle_test.test_sidebar.ShellSidebarTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "f:\dev\3x\lib\idlelib\idle_test\test_sidebar.py", line 414, in setUpClass
        cls.init_shell()
      File "f:\dev\3x\lib\idlelib\idle_test\test_sidebar.py", line 429, in init_shell
        cls.shell = cls.flist.open_shell()
      File "f:\dev\3x\lib\idlelib\pyshell.py", line 334, in open_shell
        self.pyshell = PyShell(self)
      File "f:\dev\3x\lib\idlelib\pyshell.py", line 890, in __init__
        OutputWindow.__init__(self, flist, None, None)
      File "f:\dev\3x\lib\idlelib\outwin.py", line 79, in __init__
        EditorWindow.__init__(self, *args)
      File "f:\dev\3x\lib\idlelib\editor.py", line 342, in __init__
        self.update_menu_state('options', '*ode*ontext', 'disabled')
      File "f:\dev\3x\lib\idlelib\editor.py", line 488, in update_menu_state
        menuitem.entryconfig(index, state=state)
      File "f:\dev\3x\lib\tkinter\__init__.py", line 3383, in entryconfigure
        return self._configure(('entryconfigure', index), cnf, kw)
      File "f:\dev\3x\lib\tkinter\__init__.py", line 1660, in _configure
        self.tk.call(_flatten((self._w, cmd)) + self._options(cnf))
    _tkinter.TclError: bad menu entry index "*ode*ontext"

    One of the things that overrideRootMenu does is to move the option setting item, above the code context item, to the application submenu. I don't understand why this caused the failure above. However, removing the setupApp call fixed this failure also. The first PR does this.

    With this fix, test_idle runs 6 times with leak report
    test_idle leaked [684, 684, 684] references, sum=2052
    test_idle leaked [282, 282, 282] memory blocks, sum=846

    With ShellSidebarTest skipped, the result is only slightly better.
    test_idle leaked [684, 684, 665] references, sum=2033
    test_idle leaked [282, 282, 283] memory blocks, sum=847

    @terryjreedy terryjreedy added 3.10 only security fixes labels Apr 30, 2021
    @terryjreedy
    Copy link
    Member

    New changeset a62e424 by Terry Jan Reedy in branch 'master':
    bpo-43981: Fix error in idle-test leak test (GH-25739)
    a62e424

    @terryjreedy
    Copy link
    Member

    test_squeezer.ExpandingButtonTest accounts for most of the leak.
    python -m test -R3:3 -v -ugui -m *zer.Expand* test_idle
    test_idle leaked [576, 576, 576] references, sum=1728
    test_idle leaked [282, 282, 282] memory blocks, sum=846
    All 6 methods failed. I believe the difference [108, 108, 108] [0, 0, 0] would not be a failure.

    Yesterday's patch, https://github.com/python/cpython/pull/22682/files,
    only made what seems like a trivial change in 2 of the 6 methods. Only a name was changed in squeeze.py.

    Tal, any idea why these are leaking?

    @pablogsal
    Copy link
    Member Author

    I will try to investigate today or tomorrow but notice that is very possible that the leak is somewhere else in the interpreter and is just showing in the IDLE tests.

    @pablogsal
    Copy link
    Member Author

    Taking into account the last time the builder built successully it must have happened in some of these commits:

    b38b2fa Document importlib.metadata.PackagePath.locate method (GH-25669)
    4a85718 bpo-43970: Optimize Path.cwd() in pathlib by not instantiating a class unnecessarily (GH-25699)
    15d3861 bpo-37903: IDLE: Shell sidebar with prompts (GH-22682)
    103d5e4 bpo-28254: _posixsubprocess uses PyGC_Enable/PyGC_Disable (GH-25693)
    3b52c8d bpo-43908: Add Py_TPFLAGS_IMMUTABLETYPE flag (GH-25520)
    3cc481b bpo-28254: Add a C-API for controlling the GC state (GH-25687)
    baecfbd bpo-43757: Make pathlib use os.path.realpath() to resolve symlinks in a path (GH-25264)
    859577c bpo-41559: Change PEP-612 implementation to pure Python (bpo-25449)
    c1a9535 bpo-43955: Handle the case where the distutils warning has already been triggered (GH-25675)
    4c49be7 bpo-43959: clarify the documentation of the PyContextVar C-API (GH-25671)
    fe52eb6 bpo-43961: Fix test_logging.test_namer_rotator_inheritance() (GH-25684)
    32c5a17 bpo-43962: Fix _PyInterpreterState_IDIncref() (GH-25683)
    21b02b5 bpo-43960: test_pdb resets breakpoints (GH-25673)
    db0c5b7 bpo-43776: Remove list call from args in Popen repr (GH-25338)
    f9bedb6 bpo-41486: Faster bz2/lzma/zlib via new output buffering (GH-21740)
    a5e6444 bpo-43963: Add _signal module state (GH-25676)
    5c84bb5 bpo-37751: Update codecs.register() doc. (GH-25643)

    @pablogsal
    Copy link
    Member Author

    I checked and reverting commit 15d3861 fixes the leaks.

    @pablogsal
    Copy link
    Member Author

    Unfortunately given how close are we to the beta release and per our buildbot policy (https://discuss.python.org/t/policy-to-revert-commits-on-buildbot-failure/404) I may need to revert commit 15d3861 to avoid masking other issues.

    @pablogsal
    Copy link
    Member Author

    New changeset 6689e45 by Pablo Galindo in branch 'master':
    bpo-43981: Fix reference leaks in test_squeezer (GH-25758)
    6689e45

    @pablogsal
    Copy link
    Member Author

    Refleaks fixed by 6689e45

    @terryjreedy
    Copy link
    Member

    I was just about to try adding that exact cleanup close after finishing reading emails ;-). Sorry I didn't think of it last night. Glad it worked. The alternative would have been to skip the test case.

    @taleinat
    Copy link
    Contributor

    taleinat commented May 1, 2021

    Pablo, many thanks for catching this, analyzing it and finding a fix!

    Sorry for missing this cleanup in the first place, and moreso for not having being able to look into this when you brought it up.

    @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
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants