msg26765 - (view) |
Author: Sverker Nilsson (svenil) |
Date: 2005-10-30 21:49 |
Tkinter.Menu.delete does not delete the commands
defined for the entries it deletes. Those objects
will be retained until the menu itself is deleted.
For example, after code like this:
button = Menubutton(root, text='Window')
menu = Menu(button)
button['menu'] = menu
def command():
print 'command button pressed'
menu.add_command(command=command)
menu.delete(END)
del command
the command function will still be referenced and
kept in memory - until the menu object itself is
destroyed.
This may not always be a serious problem, but in
my case the menu was a 'Window' menu and the
command was a method on a window top level widget,
so retaining a pointer to it after deleting the
menu entry kept a reference to that entire window,
with any associated data.
I have figured out a possible fix that is in the
attached file test_menuleak.py that contains some
test functions.
I also changed the comment - for as far as I can
see, the second optional index is actually
INCLUDED in the range of entries deleted.
Version info
Python 2.3.3 (#2, Mar 11 2004, 19:45:43)
[GCC 2.95.2 20000220 (Debian GNU/Linux)] on linux2
I think it applies to all versions: I tested with
the latest 2.4.2 as well.
Sverker Nilsson
|
msg70550 - (view) |
Author: Robert Schuppenies (schuppenies) *  |
Date: 2008-08-01 12:19 |
The problem does still exist (Python 2.6b2).
I attached a patch for Tkinter.py which addresses this problem. It is
the same as in the first proposed fix, but adds an additional check
whether the menu item has a 'command' property.
I also removed the "INDEX2 (is included)" comment, as it is not the
desired behavior (see http://www.tcl.tk/man/tcl8.5/TkCmd/text.htm#M98).
I cannot confirm the behavior, but it should be a separate issue
nevertheless.
|
msg70831 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2008-08-07 14:36 |
The patch is fine, please apply (also to the 2.5 and 3.0 branches).
Don't forget a Misc/NEWS entry.
|
msg70971 - (view) |
Author: Robert Schuppenies (schuppenies) *  |
Date: 2008-08-10 11:32 |
Fixed in r65622. Backported to the release25-maint and merged into the
py3k branch.
|
msg71243 - (view) |
Author: Guilherme Polo (gpolo) *  |
Date: 2008-08-16 22:04 |
Uhm, this patch can cause trouble if not adapted a bit. The index method
may return None for either index1 or index2, which will cause a
TypeError in that for loop.
If code is needed to confirm this, try the following:
menu = Tkinter.Menu(tearoff=False)
menu.delete(0)
|
msg71348 - (view) |
Author: Robert Schuppenies (schuppenies) *  |
Date: 2008-08-18 16:29 |
You are right. How about the attached patch, do you see any problems
here? Tkinter seems to ignore any delete calls when either of the
indices is None, so the deletion of commands may be ignored as well. But
I couldn't find a description making this API behavior official.
And does anybody know about a test suite for the Tkinter library where
issues like these are tested?
|
msg71371 - (view) |
Author: Guilherme Polo (gpolo) *  |
Date: 2008-08-18 20:08 |
You could return if in that new if statement.
As you noted, the None argument is ignored there, that is because
_tkinter checks for a None parameter, and if it happens to be a None, it
then stops processing new arguments, so this is not directly related to
the delete command of the Menu widget.
Regarding a test suite.. it would be very nice to get one for Tkinter.
Some weeks ago I did some googling but found no attempts for my surprise.
|
msg71372 - (view) |
Author: Guilherme Polo (gpolo) *  |
Date: 2008-08-18 20:09 |
change this:
"You could return if in that new if statement."
to:
"You could return in that new if statement.", please.
|
msg71446 - (view) |
Author: Robert Schuppenies (schuppenies) *  |
Date: 2008-08-19 16:58 |
I was thinking about returning in that new if statement, too, but
decided not too. The reason is that I didn't want to anticipate _tkinter
implementations, which may change (although not likely, still possible).
Also, with the third beta tomorrow, I am not sure if somebody will find
the time to approve the patch in time.
|
msg71447 - (view) |
Author: Guilherme Polo (gpolo) *  |
Date: 2008-08-19 17:05 |
If this needs approval of someone else, and such thing doesn't happen in
time then the earlier patch should be reverted.
|
msg71604 - (view) |
Author: Guilherme Polo (gpolo) *  |
Date: 2008-08-21 03:33 |
Well, beta3 was released and the problem remained there.
Robert, I believe MvL assigned this to you for a reason.. I'm a bit
stressed but what I'm trying to say is that you could have decided about
this issue yourself and you decided to keep the broken patch there.
|
msg71610 - (view) |
Author: Robert Schuppenies (schuppenies) *  |
Date: 2008-08-21 07:29 |
I am sry that you see it that way, I do not. I was given commit access
solely for gsoc purposes and committing changes before a late release
without review from a committer IMHO violates strict policies. I tried
to get somebody to review the patch twice on the #python-dev channel,
but was ignored. Maybe I should have made more fuss about it. Also,
since it is still a beta, it is not the end of the world. I don't like
it either but take the blame now.
|
msg71674 - (view) |
Author: Benjamin Peterson (benjamin.peterson) *  |
Date: 2008-08-21 20:05 |
I think the new patch looks fine and should be applied.
|
msg71729 - (view) |
Author: Robert Schuppenies (schuppenies) *  |
Date: 2008-08-22 08:29 |
Fixed in r65971. Backported to the release25-maint and merged into the
py3k branch.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:13 | admin | set | github: 42536 |
2008-08-22 08:29:54 | schuppenies | set | status: open -> closed resolution: fixed messages:
+ msg71729 |
2008-08-21 20:05:04 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages:
+ msg71674 |
2008-08-21 19:05:40 | gpolo | set | keywords:
+ needs review, - patch |
2008-08-21 07:29:56 | schuppenies | set | messages:
+ msg71610 |
2008-08-21 03:33:00 | gpolo | set | messages:
+ msg71604 |
2008-08-19 17:05:43 | gpolo | set | messages:
+ msg71447 |
2008-08-19 16:58:56 | schuppenies | set | messages:
+ msg71446 |
2008-08-18 20:09:16 | gpolo | set | messages:
+ msg71372 |
2008-08-18 20:08:11 | gpolo | set | messages:
+ msg71371 |
2008-08-18 16:29:59 | schuppenies | set | files:
+ tkinter_menu-error.patch messages:
+ msg71348 |
2008-08-16 22:04:04 | gpolo | set | status: closed -> open nosy:
+ gpolo resolution: fixed -> (no value) messages:
+ msg71243 |
2008-08-10 11:32:06 | schuppenies | set | status: open -> closed resolution: accepted -> fixed messages:
+ msg70971 |
2008-08-07 14:36:08 | loewis | set | assignee: loewis -> schuppenies resolution: accepted messages:
+ msg70831 |
2008-08-01 12:19:33 | schuppenies | set | files:
+ tkinter_menuleak.patch nosy:
+ schuppenies messages:
+ msg70550 keywords:
+ patch |
2005-10-30 21:49:23 | svenil | create | |