Patches item #1524639, was opened at 2006-07-18 17:00 Message generated for change (Comment added) made by grahamh You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1524639&group_id=5470
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Tkinter Group: Python 2.5 Status: Open Resolution: None Priority: 5 Submitted By: Graham Horler (grahamh) Assigned to: Martin v. Löwis (loewis) Summary: Fix Tkinter Tcl-commands memory-leaks Initial Comment: Fix 8 memory-leaks by cleaning up created Tcl commands automatically. I attach a patch against Tkinter 47021. === Long explanation === I was bitten by a memory leak in Tkinter - 25MB per day on a long-running process. A net search found a couple unrelated Tkinter leaks, and gave me some clues. Investigation using the tracing feature in tkleak.py (see link) found the bug. I searched for more similar leaks, and fixed them too. The reasoning for patch #1121234 gives the reason for the changes to _register() and deletecommand(). See http://www.uk.debian.org/~graham/python/tkleak.py for my leak tracing and test script. ---------------------------------------------------------------------- >Comment By: Graham Horler (grahamh) Date: 2006-07-24 10:29 Message: Logged In: YES user_id=543663 > The patch looks wrong. Why are you deleting the > _tkcommands of the master widget if the variable > is deleted? The master could live much longer, > and the commands might still be needed. AIUI, Tkinter callbacks (Button and Scrollbar commands, Text [xy]view, event, protocol and variable bindings, after callbacks etc) should always be given a Python callable, and not the Tcl name of an already bound Python command. Example: b1 = Button(root, command=pressed) b2 = Button(root, command=b1['command']) # Share command b1.destroy() b2.invoke() # Callback is invalid, and rightly so Given that, when a Variable is deleted, is needs to clean up the Tcl commands it created, as no other widget has a reference to them, and they are no longer needed, and there will be no other way of deleting them. By the time __del__() deletes any 'u' callbacks, they have already been called, as globalunsetvar is called first, so this is safe. > What is the purpose of the changes to trace_variable? > You are supposed to invoke trace_vdelete to release the > callback. Is it the aim for Tkinter to work in a similar way to Python's reference counted and garbage collected mem-man? I assume it is, so the changes track commands associated with the variable, for cleanup on collection. See next comment below for explanation of TclError. > Why are you catching TclError in so many cases? Under what > circumstances can you even get an error? The following all raise TclError: w.after("spam", eggs) # bad argument "spam": must be cancel, idle, info, or a number w.bind("<spam>", eggs) # bad event type or keysym "spam" w.bind_all("<spam>", eggs) # bad event type or keysym "spam" w.bind_class("Button", "<spam>", eggs) # bad event type or keysym "spam" w.config(spam=eggs) # unknown option "-spam" v.trace_variable("spam", eggs) # bad operations "spam": should be one or more of rwu When the exception is raised, the 'eggs' Tcl command for the 'spam' callback has already been created. The exception means the "command-id" is never returned for use in cleaning up manually. So, the patch cleans up on exception automatically. I don't think the argument that it's OK for incorrect use to result in memory leaks holds any water :-) I may of course be missing the obvious. > I think I would prefer if this patch was split into many > individual ones, each one fixing only a single > bug (assuming there are multiple bugs in Tkinter). Sorry for the rushed patch, I noticed the release schedule for Python2.5 and just put together the patch from monkey-patch code I had already been using, tested it and uploaded. The tkleak.py program mentioned tests all the fixes, but I did not have time to add them to the Python test suite. I realise you probably have many demands on your time (fielding loads of patches like this one :-), like I do (baby imminent! work demands etc.), so I hope the explanations above help (I should have explained all this before), if not I may be able to submit separate patches and tests at a later date. Many thanks, Graham Horler ---------------------------------------------------------------------- Comment By: Martin v. Löwis (loewis) Date: 2006-07-24 07:15 Message: Logged In: YES user_id=21627 The patch looks wrong. Why are you deleting the _tkcommands of the master widget if the variable is deleted? The master could live much longer, and the commands might still be needed. What is the purpose of the changes to trace_variable? You are supposed to invoke trace_vdelete to release the callback. Why are you catching TclError in so many cases? Under what circumstances can you even get an error? I think I would prefer if this patch was split into many individual ones, each one fixing only a single bug (assuming there are multiple bugs in Tkinter). ---------------------------------------------------------------------- Comment By: Graham Horler (grahamh) Date: 2006-07-19 11:33 Message: Logged In: YES user_id=543663 I fixed a bug in the original patch: when destroying a window which had Variable instances attached which in turn had trace commands bound, Tkinter.py was trying to delete the commands twice, as the command was mentioned in Variable instance _tclCommands and _root()._tcl_Commands. Also fixed 4 more leaks occurring when TclError is raised after a callback has been created. I have added 6 tests to tkleak.py to test the additional 4 fixes. The patch is against 50704, hope it helps. Thanks for your hard work, and for accepting my other Tkinter.py patch. ---------------------------------------------------------------------- Comment By: Graham Horler (grahamh) Date: 2006-07-18 17:18 Message: Logged In: YES user_id=543663 The python version I am using in production is 2.1 (for historical reasons), however the memory leak bugs are in 2.1 through to 2.5. I can generate a patch against any of the older versions (e.g. 2.4) if anyone is interested. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1524639&group_id=5470 _______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches
