classification
Title: test_idle is leaking references
Type: Stage: resolved
Components: IDLE Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: pablogsal, taleinat, terry.reedy
Priority: release blocker Keywords: patch

Created on 2021-04-29 21:29 by pablogsal, last changed 2021-05-01 09:57 by taleinat. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25739 merged terry.reedy, 2021-04-30 03:28
PR 25758 merged pablogsal, 2021-04-30 17:56
Messages (13)
msg392353 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-29 21:29
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
msg392354 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-29 21:29
Marking this as a release blocker
msg392376 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-04-30 03:24
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
msg392377 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-04-30 03:52
New changeset a62e424de0c394cda178a8d934d06f0559b5e28d by Terry Jan Reedy in branch 'master':
bpo-43981: Fix error in idle-test leak test (GH-25739)
https://github.com/python/cpython/commit/a62e424de0c394cda178a8d934d06f0559b5e28d
msg392381 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-04-30 04:28
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?
msg392424 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-30 12:01
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.
msg392467 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-30 16:24
Taking into account the last time the builder built successully it must have happened in some of these commits:

b38b2fa021 Document importlib.metadata.PackagePath.locate method (GH-25669)
4a85718212 bpo-43970: Optimize Path.cwd() in pathlib by not instantiating a class unnecessarily (GH-25699)
15d3861856 bpo-37903: IDLE: Shell sidebar with prompts (GH-22682)
103d5e420d bpo-28254: _posixsubprocess uses PyGC_Enable/PyGC_Disable (GH-25693)
3b52c8d66b bpo-43908: Add Py_TPFLAGS_IMMUTABLETYPE flag (GH-25520)
3cc481b9de bpo-28254: Add a C-API for controlling the GC state (GH-25687)
baecfbd849 bpo-43757: Make pathlib use os.path.realpath() to resolve symlinks in a path (GH-25264)
859577c249 bpo-41559: Change PEP 612 implementation to pure Python (#25449)
c1a9535989 bpo-43955: Handle the case where the distutils warning has already been triggered (GH-25675)
4c49be7668 bpo-43959: clarify the documentation of the PyContextVar C-API (GH-25671)
fe52eb6219 bpo-43961: Fix test_logging.test_namer_rotator_inheritance() (GH-25684)
32c5a17444 bpo-43962: Fix _PyInterpreterState_IDIncref() (GH-25683)
21b02b5f40 bpo-43960: test_pdb resets breakpoints (GH-25673)
db0c5b786d bpo-43776: Remove list call from args in Popen repr (GH-25338)
f9bedb630e bpo-41486: Faster bz2/lzma/zlib via new output buffering (GH-21740)
a5e64444e6 bpo-43963: Add _signal module state (GH-25676)
5c84bb506a bpo-37751: Update `codecs.register()` doc. (GH-25643)
msg392474 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-30 16:42
I checked and reverting commit 15d3861856 fixes the leaks.
msg392475 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-30 16:44
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 15d3861856 to avoid masking other issues.
msg392494 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-30 18:34
New changeset 6689e45dfee75d756c540ff0946ebf0ae8847f43 by Pablo Galindo in branch 'master':
bpo-43981: Fix reference leaks in test_squeezer (GH-25758)
https://github.com/python/cpython/commit/6689e45dfee75d756c540ff0946ebf0ae8847f43
msg392495 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-04-30 18:34
Refleaks fixed by 6689e45dfee75d756c540ff0946ebf0ae8847f43
msg392512 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2021-04-30 20:49
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.
msg392579 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2021-05-01 09:57
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.
History
Date User Action Args
2021-05-01 09:57:36taleinatsetmessages: + msg392579
2021-04-30 20:49:07terry.reedysetmessages: + msg392512
2021-04-30 18:34:51pablogsalsetmessages: + msg392495
2021-04-30 18:34:36pablogsalsetmessages: + msg392494
2021-04-30 18:34:35pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-04-30 17:56:37pablogsalsetstage: patch review
pull_requests: + pull_request24448
2021-04-30 16:44:40pablogsalsetmessages: + msg392475
2021-04-30 16:42:58pablogsalsetmessages: + msg392474
2021-04-30 16:24:12pablogsalsetmessages: + msg392467
2021-04-30 12:01:42pablogsalsetmessages: + msg392424
2021-04-30 04:28:19terry.reedysetnosy: + taleinat

messages: + msg392381
stage: patch review -> (no value)
2021-04-30 03:52:55terry.reedysetmessages: + msg392377
2021-04-30 03:28:04terry.reedysetkeywords: + patch
stage: patch review
pull_requests: + pull_request24430
2021-04-30 03:24:44terry.reedysetmessages: + msg392376
versions: + Python 3.10
2021-04-29 21:29:30pablogsalsetmessages: + msg392354
2021-04-29 21:29:20pablogsalcreate