GNOME Bugzilla – Bug 649500
leak in theme parser
Last modified: 2011-05-11 18:15:14 UTC
==13839== 450 bytes in 18 blocks are definitely lost in loss record 4,884 of 5,648 ==13839== at 0x4006D69: malloc (vg_replace_malloc.c:236) ==13839== by 0x5191BCA: standard_malloc (gmem.c:88) ==13839== by 0x519201F: g_malloc (gmem.c:164) ==13839== by 0x51A9CF6: g_strndup (gstrfuncs.c:166) ==13839== by 0x413D835: meta_draw_spec_new (theme.c:1840) ==13839== by 0x4130B00: parse_draw_op_element (theme-parser.c:2717) ==13839== by 0x4132A99: start_element_handler (theme-parser.c:3603) ==13839== by 0x518EE5B: emit_start_element (gmarkup.c:985) ==13839== by 0x5190647: g_markup_parse_context_parse (gmarkup.c:1325) ==13839== by 0x412CCC8: load_theme (theme-parser.c:4224) ==13839== by 0x4134632: meta_theme_load (theme-parser.c:4326) ==13839== by 0x414A8A8: ??? (in /src/build/jhbuild/lib/libmutter.so.0.0.0) ==13839== 784 bytes in 56 blocks are definitely lost in loss record 5,139 of 5,648 ==13839== at 0x4006D69: malloc (vg_replace_malloc.c:236) ==13839== by 0x5191BCA: standard_malloc (gmem.c:88) ==13839== by 0x519201F: g_malloc (gmem.c:164) ==13839== by 0x51A9CF6: g_strndup (gstrfuncs.c:166) ==13839== by 0x413D835: meta_draw_spec_new (theme.c:1840) ==13839== by 0x412F0E8: parse_draw_op_element (theme-parser.c:1821) ==13839== by 0x4132A99: start_element_handler (theme-parser.c:3603) ==13839== by 0x518EE5B: emit_start_element (gmarkup.c:985) ==13839== by 0x5190647: g_markup_parse_context_parse (gmarkup.c:1325) ==13839== by 0x412CCC8: load_theme (theme-parser.c:4224) ==13839== by 0x4134632: meta_theme_load (theme-parser.c:4326) ==13839== by 0x414A8A8: ??? (in /src/build/jhbuild/lib/libmutter.so.0.0.0) ==13839== 840 bytes in 60 blocks are definitely lost in loss record 5,158 of 5,648 ==13839== at 0x4006D69: malloc (vg_replace_malloc.c:236) ==13839== by 0x5191BCA: standard_malloc (gmem.c:88) ==13839== by 0x519201F: g_malloc (gmem.c:164) ==13839== by 0x51A9CF6: g_strndup (gstrfuncs.c:166) ==13839== by 0x413D835: meta_draw_spec_new (theme.c:1840) ==13839== by 0x412EE6E: parse_draw_op_element (theme-parser.c:1744) ==13839== by 0x4132A99: start_element_handler (theme-parser.c:3603) ==13839== by 0x518EE5B: emit_start_element (gmarkup.c:985) ==13839== by 0x5190647: g_markup_parse_context_parse (gmarkup.c:1325) ==13839== by 0x412CCC8: load_theme (theme-parser.c:4224) ==13839== by 0x4134632: meta_theme_load (theme-parser.c:4326) ==13839== by 0x414A8A8: ??? (in /src/build/jhbuild/lib/libmutter.so.0.0.0)
i haven't looked too deeply to see if this /is/ the problem, but it struck me as a potential cause of the problem when scanning through the code... ==13839== by 0x413D835: meta_draw_spec_new (theme.c:1840) is around here: next->type = POS_TOKEN_VARIABLE;• next->d.v.name = g_strndup (start, p - start);• which gets cleaned up in free_tokens, but only if next->type is POS_TOKEN_VARIABLE: if (tokens[i].type == POS_TOKEN_VARIABLE)• g_free (tokens[i].d.v.name);• But there are places in the code that check if a token is of type POS_TOKEN_VARIABLE and then change its type without doing any clean up of the allocation: gboolean• meta_theme_replace_constants (MetaTheme *theme,• PosToken *tokens,• int n_tokens,• GError **err)• {• ... for (i = 0; i < n_tokens; i++)• {• PosToken *t = &tokens[i];␣␣␣␣␣␣• • if (t->type == POS_TOKEN_VARIABLE)• {• ... t->type = POS_TOKEN_INT;• .. etc. So that may be the source of the leak.
Created attachment 187324 [details] [review] theme.c: Squash memory leak When converting a token to a different type, we need to free its string. ==13839== 784 bytes in 56 blocks are definitely lost in loss record 5,139 of 5,648 ==13839== at 0x4006D69: malloc (vg_replace_malloc.c:236) ==13839== by 0x5191BCA: standard_malloc (gmem.c:88) ==13839== by 0x519201F: g_malloc (gmem.c:164) ==13839== by 0x51A9CF6: g_strndup (gstrfuncs.c:166) ==13839== by 0x413D835: meta_draw_spec_new (theme.c:1840) ==13839== by 0x412F0E8: parse_draw_op_element (theme-parser.c:1821) ==13839== by 0x4132A99: start_element_handler (theme-parser.c:3603) ==13839== by 0x518EE5B: emit_start_element (gmarkup.c:985) ==13839== by 0x5190647: g_markup_parse_context_parse (gmarkup.c:1325) ==13839== by 0x412CCC8: load_theme (theme-parser.c:4224) ==13839== by 0x4134632: meta_theme_load (theme-parser.c:4326) ==13839== by 0x414A8A8: ??? (in /src/build/jhbuild/lib/libmutter.so.0.0.0)
Review of attachment 187324 [details] [review]: - No valgrind trace in the commit message (this is a universal comment that applies forever to all patches to modules I maintain :-) Otherwise, patch looks fine to me 3-0 and master (though compared to IRC discussion, I do consider this a trivial leak, since it's bounded in normal operation, and it's been there since ~2007)
Same fix already done in metacity - see commit bc14af3aaa0cd4c51d4558209adf2fee9878b071
Attachment 187324 [details] pushed as d0414a3 - theme.c: Squash memory leak