GNOME Bugzilla – Bug 86586
Script-fu leaks memory.
Last modified: 2009-08-15 18:40:50 UTC
Actually it leaks a lot of memory. in 4 hours, I saw a script-fu up to 96 meg. Mind you, it was working very hard! Included after this are some patches... It does not fix all the problems. Cameron
Created attachment 9477 [details] [review] free memory on early return. script-fu.c and script-fu-server.c
We need to call gimp_destroy_paramdefs() on the params and return_values returned from gimp_query_procedure(). Cleaning up with g_free() will leak lots of memory. I'd appreciate if you could change this and create a new patch. And please try to avoid C++ style comments.
For GIMP-1.3 we should consider to allow to pass NULL pointers to gimp_procedural_db_proc_info() for the values we are not interested in. This would avoid unnecessary memory allocation and would reduce the risk of memory leaks.
Looking at your patch a little more I think we should introduce a cleanup label at the end of the function that frees all the memory and use a goto to jump there in case of errors. That way we can avoid to duplicate the code that frees the allocated resources at each and every return point. The GimpParam args array should be allocated using g_new0() and needs to be freed using gimp_params_destroy(). If you use g_free() you'll leak quite a bunch of memory if any string or array types have been allocated.
*** Bug 108966 has been marked as a duplicate of this bug. ***
Created attachment 15651 [details] [review] Solves the leaks as stated in bug #108966.
Applied to the HEAD branch. Should be considered for 1.2 as well. Does this close the bug-report or are there obviously more leaks? 2003-04-12 Sven Neumann <sven@gimp.org> * plug-ins/script-fu/siod-wrapper.c (marshall_proc_db_call): applied a patch from Pedro Gimero that plugs a memleak in Script-Fu.
There's still the problem of leaks on early return that the OP tried to address in his patch. Of course, it would be even better to have a function like gimp_procedural_db_get_proc_api that does not allocate useless stuff like names or descriptions; this would also make PDB calls faster. But that's just a thought.
Here's a backport of the previous patch, to solve some of the leaks in 1.2. Leaks in error handling are still pending in both branches.
Created attachment 16475 [details] [review] Backport of the previous patch
Please commit. I don't think we should care about leaks on early returns in the stable branch. The changes would likely introduce new bugs.
Fully agree. Indeed I had already patched 1.3 in my local tree but wasn't happy with the result. The patch above is now applied to 1.2: 2003-05-13 Pedro Gimeno * plug-ins/script-fu/script-fu.c: Merged a partial fix for bug #86586 (memory leaks in script-fu) from HEAD. * tools/pdbgen/pdb/color.pdb (histogram): Call gimp_histogram_free when no longer needed. * app/color_cmds.c: Regenerated. As the remaining issues are not going to be fixed in 1.2 and constitute just a mild cause of leaks, I'm resolving this bug as FIXED.
The fix is part of the stable release 1.2.4. Closing this bug.