classification
Title: [2.7] test_ttk_guionly doesn't destroy all widgets on Python 2.7
Type: resource usage Stage: resolved
Components: Tkinter Versions: Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: gpolo, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2017-08-08 00:46 by vstinner, last changed 2017-08-10 15:03 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
widget_instance.patch vstinner, 2017-08-08 00:46
fix_destroy.patch vstinner, 2017-08-08 00:46
widget_instance-master.patch vstinner, 2017-08-08 12:41 review
widget_instance-2.7-v2.patch vstinner, 2017-08-08 13:04
Pull Requests
URL Status Linked Edit
PR 3025 merged vstinner, 2017-08-08 12:48
PR 3026 merged vstinner, 2017-08-08 13:00
PR 3030 merged vstinner, 2017-08-08 23:47
Messages (12)
msg299880 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-08 00:46
While working on bpo-31068, I wrote attached widget_instance.patch to check if test_ttk_guionly really removes all widgets. Bad news: it doesn't. Example:

haypo@selma$ ./python -m test  -u all test_ttk_guionly
(...)
test test_ttk_guionly crashed -- <type 'exceptions.Exception'>: leaking 69 widgets
(...)

Attached fix_destroy.patch is my attempt to fix this issue.
msg299881 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-08 00:47
I only tested Python 2.7. I didn't check if other branches are affected.
msg299899 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-08-08 05:41
See issue25130. Maybe forcing garbage collecting will help.
msg299902 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-08 10:20
My widget_instance.patch calls test_support.gc_collect() and removes all
widgets from the root window. It's not enough. That's why I proposed to modify LabeledScale.destroy().
msg299913 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-08 12:39
On master, LabeledScaleTest.test_initialization() leaks a widget in the root widget, even after root.destroy(). The problem is that LabeledScaleTest.__del__() doesn't call Frame.destroy(self) if self._variable is not set. So master has at least this bug.
msg299914 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-08 12:41
widget_instance-master.patch: Check that Tkinter.Widget.destroy() removes all children and check that test_ttk_guionly doesn't leak widgets.
msg299915 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-08 12:49
Example of failure on master using widget_instance-master.patch:

haypo@selma$ ./python -m test -v -u all test_ttk_guionly 

======================================================================
ERROR: test_initialization (tkinter.test.test_ttk.test_extensions.LabeledScaleTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/haypo/prog/python/master/Lib/tkinter/test/test_ttk/test_extensions.py", line 14, in tearDown
    super().tearDown()
  File "/home/haypo/prog/python/master/Lib/tkinter/test/support.py", line 40, in tearDown
    w.destroy()
  File "/home/haypo/prog/python/master/Lib/tkinter/__init__.py", line 2305, in destroy
    % self.children)
Exception: destroy() doesn't clear all children: {'!labeledscale2': <tkinter.ttk.LabeledScale object .!frame.!labeledscale2>}


My https://github.com/python/cpython/pull/3025 PR fixes LabeledScale and OptionMenu destroy() method in the master branch.
msg299916 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-08 13:04
widget_instance-2.7-v2.patch: Updated patch for Python 2.7 to debug test_ttk_guionly.

https://github.com/python/cpython/pull/3026 fix LabeledScale and OptionMenu destroy() method for Python 2.7.
msg299938 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-08 17:15
New changeset 56e162ad5c5d3effe9b4f05d0179e1b6a2a2d9b8 by Victor Stinner in branch '2.7':
ttk: fix LabeledScale and OptionMenu destroy() method (#3026)
https://github.com/python/cpython/commit/56e162ad5c5d3effe9b4f05d0179e1b6a2a2d9b8
msg299940 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-08 17:41
New changeset cd7e9c1b67d3d07ee03e0a79af2791c19031cecb by Victor Stinner in branch 'master':
ttk: fix LabeledScale and OptionMenu destroy() method (#3025)
https://github.com/python/cpython/commit/cd7e9c1b67d3d07ee03e0a79af2791c19031cecb
msg299977 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-09 08:48
New changeset 33460fa7e0bd126bee739a66e1228665dc22e70f by Victor Stinner in branch '3.6':
ttk: fix LabeledScale and OptionMenu destroy() method (#3025) (#3030)
https://github.com/python/cpython/commit/33460fa7e0bd126bee739a66e1228665dc22e70f
msg300097 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-10 15:03
Copy of interesting comments: https://github.com/python/cpython/pull/3025

serhiy-storchaka: "Parent's destroy() now is called even if this destroy() already was called. I.e. it can be called twice."

haypo: "Yes, it's a deliberate choice. All other ttk widgets now have the same behaviour."

--

Ok, the bug is now fixed in 2.7, 3.6 and 3.7 (master) branches. I close the issue.
History
Date User Action Args
2017-08-10 15:03:03vstinnersetstatus: open -> closed
versions: + Python 3.6, Python 3.7
messages: + msg300097

resolution: fixed
stage: resolved
2017-08-09 08:48:16vstinnersetmessages: + msg299977
2017-08-08 23:47:05vstinnersetpull_requests: + pull_request3065
2017-08-08 17:41:23vstinnersetmessages: + msg299940
2017-08-08 17:15:55vstinnersetmessages: + msg299938
2017-08-08 13:04:49vstinnersetfiles: + widget_instance-2.7-v2.patch

messages: + msg299916
2017-08-08 13:00:43vstinnersetpull_requests: + pull_request3060
2017-08-08 12:49:47vstinnersetmessages: + msg299915
2017-08-08 12:48:21vstinnersetpull_requests: + pull_request3059
2017-08-08 12:41:40vstinnersetfiles: + widget_instance-master.patch

messages: + msg299914
2017-08-08 12:39:46vstinnersetmessages: + msg299913
2017-08-08 10:20:27vstinnersetmessages: + msg299902
2017-08-08 05:41:09serhiy.storchakasetmessages: + msg299899
2017-08-08 01:28:41vstinnersetnosy: + gpolo
2017-08-08 00:48:35vstinnersetnosy: + serhiy.storchaka
2017-08-08 00:47:23vstinnersetmessages: + msg299881
title: test_ttk_guionly doesn't destroy all widgets -> [2.7] test_ttk_guionly doesn't destroy all widgets on Python 2.7
2017-08-08 00:46:59vstinnersetfiles: + fix_destroy.patch
2017-08-08 00:46:39vstinnercreate