GNOME Bugzilla – Bug 549952
strut leak
Last modified: 2008-09-16 02:31:10 UTC
I'm seeing a small leak in valgrind: ==6624== 80 bytes in 4 blocks are definitely lost in loss record 6,623 of 7,262 ==6624== at 0x4028AEE: malloc (vg_replace_malloc.c:207) ==6624== by 0x90EA33: g_malloc (in /lib/libglib-2.0.so.0.1707.0) ==6624== by 0x808C2D3: ensure_work_areas_validated (workspace.c:518) ==6624== by 0x808C852: meta_workspace_get_work_area_for_xinerama (workspace.c:669) ==6624== by 0x8081CFC: meta_window_get_work_area_for_xinerama (window.c:7519) ==6624== by 0x805B9FB: meta_window_constrain (constraints.c:392) ==6624== by 0x8084664: meta_window_move_resize_internal (window.c:3262) ==6624== by 0x80853D2: meta_window_move_resize_request (window.c:4584) ==6624== by 0x808555A: meta_window_configure_request (window.c:4618) ==6624== by 0x80625DB: event_callback (display.c:2132) ==6624== by 0x80AB3BE: filter_func (ui.c:83) Looking at the code, ensure_work_areas_validated copies the struts that are placed on the all_struts list: for (s_iter = win->struts; s_iter != NULL; s_iter = s_iter->next) { MetaStrut *cpy = g_new (MetaStrut, 1); *cpy = *((MetaStrut *)s_iter->data); workspace->all_struts = g_slist_prepend (workspace->all_struts, cpy); } but workspace_free just throws away the list: if (!workspace->work_areas_invalid) { g_slist_free (workspace->all_struts);
Oh dear, that's my fault: this was introduced in bug 468075, a simple patch, and I should have spotted the leak when I reviewed it: http://svn.gnome.org/viewvc/metacity?view=revision&revision=3817 This means we'll have to figure out how to do this properly...
Created attachment 117671 [details] [review] Free a It seems to me that it's actually that r3817 just didn't go far enough; it makes sure that we keep a copy of the struts, but it didn't remember to run through the list later freeing them. Do you still see the problem after this patch is applied?
Sorry about that. I had seen that g_list_free() was called, but mistakenly understood that it would also free the data pointed by each node. Indeed, I think your patch should be sufficient to fix the leaks.
Oh, I forgot to add prototypes, I'm sorry-- it was just supposed to be a mockup. I'll add them.
In trunk now (with fixes). http://svn.gnome.org/viewvc/metacity?rev=3840&view=rev
Still seeing this in 2.23.610 ==23147== 80 bytes in 4 blocks are definitely lost in loss record 5,414 of 6,118 ==23147== at 0x4006AEE: malloc (vg_replace_malloc.c:207) ==23147== by 0xA88873: g_malloc (gmem.c:131) ==23147== by 0x808BF03: ensure_work_areas_validated (workspace.c:518) ==23147== by 0x808C482: meta_workspace_get_work_area_for_xinerama (workspace.c:669) ==23147== by 0x8081A6C: meta_window_get_work_area_for_xinerama (window.c:7519) ==23147== by 0x805B9BA: meta_window_constrain (constraints.c:392) ==23147== by 0x80842D4: meta_window_move_resize_internal (window.c:3262) ==23147== by 0x8085022: meta_window_move_resize_request (window.c:4584) ==23147== by 0x80851AA: meta_window_configure_request (window.c:4618) ==23147== by 0x806243B: event_callback (display.c:2132) ==23147== by 0x80AAFCE: filter_func (ui.c:83)
Probably the fix was commited only to the trunk, and not to the 2.23 branch...
I think you're right. However, since we're in hard freeze, I've mailed the release team to ask permission to apply the patch to 2.23.