| msg52726 - (view) |
Author: Robert Hancock (robhancock) |
Date: 2007-06-05 21:47 |
| We have a Python application which displays a GUI using Tkinter and Pmw. We have seen that over time the memory usage of the app seems to continuously increase and eventually use up all of the system's RAM. I ran the app under Valgrind, and this leak seems to account for most of the memory that it detects as being leaked: ==31141== 2,630,208 (806,400 direct, 1,823,808 indirect) bytes in 21 blocks are definitely lost in loss record 156 of 159 ==31141== at 0x4A05809: malloc (vg_replace_malloc.c:149) ==31141== by 0x368268844A: TclThreadAllocObj (in /usr/lib64/libtcl8.4.so) ==31141== by 0x3682686BA0: Tcl_NewStringObj (in /usr/lib64/libtcl8.4.so) ==31141== by 0x8B3B258: AsObj (_tkinter.c:902) ==31141== by 0x8B3BB1C: Tkapp_CallArgs (_tkinter.c:1149) ==31141== by 0x8B3BD67: Tkapp_CallProc (_tkinter.c:1224) ==31141== by 0x36826786D4: Tcl_ServiceEvent (in /usr/lib64/libtcl8.4.so) ==31141== by 0x3682678A20: Tcl_DoOneEvent (in /usr/lib64/libtcl8.4.so) ==31141== by 0x8B3F55B: Tkapp_MainLoop (_tkinter.c:2532) ==31141== by 0x36900949D9: PyEval_EvalFrame (in /usr/lib64/libpython2.4.so.1.0) ==31141== by 0x3690095904: PyEval_EvalCodeEx (in /usr/lib64/libpython2.4.so.1.0) ==31141== by 0x369009405E: PyEval_EvalFrame (in /usr/lib64/libpython2.4.so.1.0) ==31141== by 0x3690094485: PyEval_EvalFrame (in /usr/lib64/libpython2.4.so.1.0) ==31141== by 0x3690095904: PyEval_EvalCodeEx (in /usr/lib64/libpython2.4.so.1.0) ==31141== by 0x3690095951: PyEval_EvalCode (in /usr/lib64/libpython2.4.so.1.0) ==31141== by 0x36900B1EA8: (within /usr/lib64/libpython2.4.so.1.0) ==31141== by 0x36900B3357: PyRun_SimpleFileExFlags (in /usr/lib64/libpython2.4.so.1.0) ==31141== by 0x36900B979C: Py_Main (in /usr/lib64/libpython2.4.so.1.0) ==31141== by 0x368161D8A3: (below main) (in /lib64/libc-2.5.so) Looking at the code in _tkinter.c, I am not sure how the Tkapp_CallDeallocArgs function is supposed to get called in the case where we push the request onto the Tcl interpreter thread in Tkapp_Call. That call is what would presumably release this allocated memory. This is using Python 2.4.3 and tcl/tk 8.4.13. Looking at Python SVN, there does not seem to be any relevant code changes in _tkinter.c in later versions.. |
|
|
| msg52727 - (view) |
Author: Robert Hancock (robhancock) |
Date: 2007-06-06 16:53 |
| Found a couple of issues in _tkinter.c that appear to have been causing this: -When calls were forwarded onto the Tcl interpreter thread, the Tkapp_CallDeallocArgs function was never called to clean up the converted arguments after the call. -Tcl_Condition variables were never finalized after being used. According to the Tcl documentation, if this is not done they are only cleaned up on program exit, resulting in them accumulating during runtime. I'm attaching a patch against Python 2.4.3 to fix this problem. It likely applies to later versions as well. File Added: python-2.4.3-memleak-fix.patch |
|
|
| msg52728 - (view) |
Author: Neal Norwitz (nnorwitz) *  |
Date: 2007-06-07 05:24 |
| Thanks for the report and patch! Can you attach a python program that demonstrates these bugs? I'd like to add tests for these. The memory leak looks pretty straightforward, but I don't know about the other Condition changes. |
|
|
| msg77954 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2008-12-17 02:48 |
| Sample attached for demonstrating the leak by missing a call to Tkapp_CallDeallocArgs. |
|
|
| msg80947 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-02-02 16:06 |
| I think the patch is correct. gpolo, what's your opinion? |
|
|
| msg80979 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2009-02-02 18:34 |
| Do we still want to say _tkinter.c supports tcl/tk from version 8.2 and newer ? If yes, then we should add a no-op Tcl_ConditionFinalize when using tcl older than 8.3, because that was when this function got added. Besides that, the patch looks fine to me too (except I prefer Tcl_Condition *cond over Tcl_Condition* cond). |
|
|
| msg81001 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-02-02 21:39 |
| > Do we still want to say _tkinter.c supports tcl/tk from version 8.2 and > newer ? If yes, then we should add a no-op Tcl_ConditionFinalize when > using tcl older than 8.3, because that was when this function got added. Good point. If we want to fix this in 2.6, we *have* to say that. If we ignore 2.6, and only fix 2.7, we could require a newer Tcl version. |
|
|
| msg81016 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2009-02-02 23:21 |
| It is much more important to fix the memory leak anyway, it is possible to notice real problems using test_tkleak1.py and watching the process with ps or something else provided by the platform. Besides, not calling ConditionFinalize doesn't introduce memory leaks, so it is not in par with issue's title. But I'm unsure about the existence of systems using Python as the main language which are also using tkinter and at the same time depend on tcl/tk 8.2 for some reason. So, to me, it would seem fine to start requiring tcl/tk 8.3 (or even newer!) starting with 2.7 and 3.1 |
|
|
| msg81017 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-02-02 23:33 |
| > Besides, not calling ConditionFinalize doesn't introduce memory leaks Are you sure about this? On Unix, TclpFinalizeCondition does pthread_cond_destroy(pcondPtr); ckfree((char *) pcondPtr); both of which release memory (IIUC). Likewise for Windows, except that it calls DeleteCriticalSection. |
|
|
| msg81018 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2009-02-02 23:40 |
| >> Besides, not calling ConditionFinalize doesn't introduce memory leaks > > Are you sure about this? On Unix, TclpFinalizeCondition does > > pthread_cond_destroy(pcondPtr); > ckfree((char *) pcondPtr); > > both of which release memory (IIUC). Likewise for Windows, except > that it calls DeleteCriticalSection. > I think I meant something more like "it doesn't result in memory definitely lost" because tcl will free these conditions when tcl finishes, but it does introduce memory leak in the sense that the memory isn't freed right after it is not longer used. |
|
|
| msg81063 - (view) |
Author: Robert Hancock (robhancock1) |
Date: 2009-02-03 15:14 |
| That's not really a meaningful difference, though.. if the application uses this code continuously then the conditions will pile up in memory until it fills up. |
|
|
| msg81065 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2009-02-03 16:04 |
| > That's not really a meaningful difference, though.. if the application > uses this code continuously then the conditions will pile up in memory > until it fills up. I'm not trying to discourage you to do the complete fix (for these two problems), but this API was made public only in tcl 8.3.1 as can be seen at http://tcl.cvs.sourceforge.net/viewvc/tcl/tcl/ChangeLog?revision=1.226&view=markup&pathrev=core-8-3-1 Still, the "call" fix should be backported to python 2.6 as it doesn't depend on changing supported tcl versions and is also more severe. |
|
|
| msg81087 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2009-02-03 18:58 |
| > Still, the "call" fix should be backported to python 2.6 as it doesn't > depend on changing supported tcl versions and is also more severe. That sounds like a good approach. If you can split the patch into two, backporting only one of them - go ahead! |
|
|
| msg81315 - (view) |
Author: Guilherme Polo (gpolo) * |
Date: 2009-02-06 23:33 |
| Committed now. trunk: r69376, r69377 release26-maint: r69378 py3k: r69380 release30-maint: r69381 (hand-merged, as you may notice I forgot to split the patch in two again in py3k) |
|
|