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

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.

The patch is fine, please apply (also to the 2.5 and 3.0 branches).
Don't forget a Misc/NEWS entry.

Fixed in r65622. Backported to the release25-maint and merged into the
py3k branch.

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)

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?

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.

change this:

"You could return if in that new if statement."

to:

"You could return in that new if statement.", please.

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.

If this needs approval of someone else, and such thing doesn't happen in
time then the earlier patch should be reverted.

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.

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.

I think the new patch looks fine and should be applied.

Fixed in r65971. Backported to the release25-maint and merged into the
py3k branch.