GNOME Bugzilla – Bug 682723
Leaks in ephy-embed-prefs.c:build_accept_language_headers et al
Last modified: 2017-03-20 21:38:27 UTC
==14171== 3 bytes in 1 blocks are definitely lost in loss record 130 of 13,792 ==14171== at 0x4A074CD: malloc (vg_replace_malloc.c:236) ==14171== by 0x7AA4C93: standard_malloc (gmem.c:85) ==14171== by 0x7AA4D1C: g_malloc (gmem.c:159) ==14171== by 0x7AA5022: g_malloc_n (gmem.c:400) ==14171== by 0x7ABFAED: g_strndup (gstrfuncs.c:428) ==14171== by 0x496D72: ephy_langs_sanitise (ephy-langs.c:64) ==14171== by 0x47567E: webkit_pref_callback_accept_languages (ephy-embed-prefs.c:370) ==14171== by 0x475E21: ephy_embed_prefs_init (ephy-embed-prefs.c:665) ==14171== by 0x42E6C4: main (ephy-main.c:481) ==14171== 6 bytes in 1 blocks are definitely lost in loss record 229 of 13,792 ==14171== at 0x4A074CD: malloc (vg_replace_malloc.c:236) ==14171== by 0x7AA4C93: standard_malloc (gmem.c:85) ==14171== by 0x7AA4D1C: g_malloc (gmem.c:159) ==14171== by 0x7AA5022: g_malloc_n (gmem.c:400) ==14171== by 0x7ABFAED: g_strndup (gstrfuncs.c:428) ==14171== by 0x7AC062D: g_ascii_strdown (gstrfuncs.c:1474) ==14171== by 0x475639: webkit_pref_callback_accept_languages (ephy-embed-prefs.c:365) ==14171== by 0x475E21: ephy_embed_prefs_init (ephy-embed-prefs.c:665) ==14171== by 0x42E6C4: main (ephy-main.c:481) ==14171== 6 bytes in 1 blocks are definitely lost in loss record 230 of 13,792 ==14171== at 0x4A074CD: malloc (vg_replace_malloc.c:236) ==14171== by 0x7AA4C93: standard_malloc (gmem.c:85) ==14171== by 0x7AA4D1C: g_malloc (gmem.c:159) ==14171== by 0x7AA5022: g_malloc_n (gmem.c:400) ==14171== by 0x7ABFA48: g_strdup (gstrfuncs.c:364) ==14171== by 0x4754F0: build_accept_languages_header (ephy-embed-prefs.c:322) ==14171== by 0x47568A: webkit_pref_callback_accept_languages (ephy-embed-prefs.c:372) ==14171== by 0x475E21: ephy_embed_prefs_init (ephy-embed-prefs.c:665) ==14171== by 0x42E6C4: main (ephy-main.c:481)
I tried something like this the following patch, that includes: - Setting the clear function for the GArray (not sure if needed?) - Being less "smart" about how we manage the memory in the array. But it does not work. Honestly not sure where the leak is here, just attaching this in case it's useful for anyone. diff --git a/embed/ephy-embed-prefs.c b/embed/ephy-embed-prefs.c index 554d6d5..a493386 100644 --- a/embed/ephy-embed-prefs.c +++ b/embed/ephy-embed-prefs.c @@ -291,6 +291,7 @@ build_accept_languages_header (GArray *languages) gchar *langs_str = NULL; gint delta; gint i; + GArray *languages_quality = NULL; g_return_val_if_fail (languages != NULL, NULL); @@ -304,24 +305,31 @@ build_accept_languages_header (GArray *languages) /* Set quality values for each language. */ langs = (gchar **)languages->data; + languages_quality = g_array_new (TRUE, FALSE, sizeof (char *)); + g_array_set_clear_func (languages_quality, (GDestroyNotify)g_free); + for (i = 0; langs[i] != NULL; i++) { gchar *lang = (gchar *)langs[i]; gint quality = 100 - i * delta; + char *str; if (quality > 0 && quality < 100) { gchar buf[8]; g_ascii_formatd (buf, 8, "%.2f", quality / 100.0); - langs[i] = g_strdup_printf ("%s;q=%s", lang, buf); + str = g_strdup_printf ("%s;q=%s", lang, buf); } else { /* Just dup the string in this case. */ - langs[i] = g_strdup (lang); + str = g_strdup (lang); } - g_free (lang); + + g_array_append_val (languages_quality, str); } /* Get the result string */ - if (languages->len > 0) - langs_str = g_strjoinv (", ", langs); + if (languages_quality->len > 0) + langs_str = g_strjoinv (", ", (gchar **)languages_quality->data); + + g_array_free (languages_quality, TRUE); return langs_str; } @@ -348,6 +356,7 @@ webkit_pref_callback_accept_languages (GSettings *settings, languages = g_settings_get_strv (settings, key); array = g_array_new (TRUE, FALSE, sizeof (char *)); + g_array_set_clear_func (array, (GDestroyNotify)g_free); for (i = 0; languages[i]; i++) { if (!g_strcmp0 (languages[i], "system")) {
(In reply to Xan Lopez from comment #1) > @@ -348,6 +356,7 @@ webkit_pref_callback_accept_languages (GSettings > *settings, > languages = g_settings_get_strv (settings, key); > > array = g_array_new (TRUE, FALSE, sizeof (char *)); > + g_array_set_clear_func (array, (GDestroyNotify)g_free); > > for (i = 0; languages[i]; i++) { > if (!g_strcmp0 (languages[i], "system")) { Warning: I tried this too, since I don't see where else any of the memory in the array could possibly be freed. But it caused an invalid free; I just don't see where.
Updated leak trace: Direct leak of 6 byte(s) in 1 object(s) allocated from: #0 0x2b8440be7a0a in malloc (/lib64/libasan.so.2+0x98a0a) #1 0x2b84575f13c4 in g_malloc /home/mcatanzaro/jhbuild/checkout/glib/glib/gmem.c:94 #2 0x2b845760923a in g_strndup /home/mcatanzaro/jhbuild/checkout/glib/glib/gstrfuncs.c:425 #3 0x2b8457609d03 in g_ascii_strdown /home/mcatanzaro/jhbuild/checkout/glib/glib/gstrfuncs.c:1494 #4 0x49f857 in webkit_pref_callback_accept_languages /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-embed-prefs.c:359 #5 0x49ffca in ephy_embed_prefs_init /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-embed-prefs.c:565 #6 0x2b84576116f7 in g_once_impl /home/mcatanzaro/jhbuild/checkout/glib/glib/gthread.c:604 #7 0x4a02a4 in ephy_embed_prefs_get_settings /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-embed-prefs.c:625 #8 0x4a7414 in ephy_web_view_new /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-web-view.c:2215 #9 0x422da1 in test_ephy_embed_shell_web_view_created /home/mcatanzaro/jhbuild/checkout/epiphany/tests/ephy-embed-shell-test.c:80 #10 0x2b84576103de in test_case_run /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2158 #11 0x2b84576105c9 in g_test_run_suite_internal /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2241 #12 0x2b8457610635 in g_test_run_suite_internal /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2253 #13 0x2b8457610635 in g_test_run_suite_internal /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2253 #14 0x2b8457610744 in g_test_run_suite /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2328 #15 0x2b845761077d in g_test_run /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:1596 #16 0x423000 in main /home/mcatanzaro/jhbuild/checkout/epiphany/tests/ephy-embed-shell-test.c:134 #17 0x2b84583a36ff in __libc_start_main (/lib64/libc.so.6+0x206ff) Direct leak of 3 byte(s) in 1 object(s) allocated from: #0 0x2b8440be7a0a in malloc (/lib64/libasan.so.2+0x98a0a) #1 0x2b84575f13c4 in g_malloc /home/mcatanzaro/jhbuild/checkout/glib/glib/gmem.c:94 #2 0x2b845760923a in g_strndup /home/mcatanzaro/jhbuild/checkout/glib/glib/gstrfuncs.c:425 #3 0x4e1d8a in ephy_langs_sanitise /home/mcatanzaro/jhbuild/checkout/epiphany/lib/ephy-langs.c:64 #4 0x49f8d2 in webkit_pref_callback_accept_languages /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-embed-prefs.c:364 #5 0x49ffca in ephy_embed_prefs_init /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-embed-prefs.c:565 #6 0x2b84576116f7 in g_once_impl /home/mcatanzaro/jhbuild/checkout/glib/glib/gthread.c:604 #7 0x4a02a4 in ephy_embed_prefs_get_settings /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-embed-prefs.c:625 #8 0x4a7414 in ephy_web_view_new /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-web-view.c:2215 #9 0x422da1 in test_ephy_embed_shell_web_view_created /home/mcatanzaro/jhbuild/checkout/epiphany/tests/ephy-embed-shell-test.c:80 #10 0x2b84576103de in test_case_run /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2158 #11 0x2b84576105c9 in g_test_run_suite_internal /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2241 #12 0x2b8457610635 in g_test_run_suite_internal /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2253 #13 0x2b8457610635 in g_test_run_suite_internal /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2253 #14 0x2b8457610744 in g_test_run_suite /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:2328 #15 0x2b845761077d in g_test_run /home/mcatanzaro/jhbuild/checkout/glib/glib/gtestutils.c:1596 #16 0x423000 in main /home/mcatanzaro/jhbuild/checkout/epiphany/tests/ephy-embed-shell-test.c:134 #17 0x2b84583a36ff in __libc_start_main (/lib64/libc.so.6+0x206ff)
The following fixes have been pushed: 4973918 embed-prefs: free variable a bit sooner 6bae3a1 embed-prefs: Fix memory leak when setting languages
Created attachment 344499 [details] [review] embed-prefs: free variable a bit sooner This does not fix anything, but it makes the function a bit easier to read by clarifying that languages is not used again later on. The code is sufficiently confusing that this seems beneficial here.
Created attachment 344500 [details] [review] embed-prefs: Fix memory leak when setting languages This was a tough one. It's a GArray rather than a GPtrArray, which led me on a wild goose chase trying to set a clear function for the array... the clear function is not allowed to actually free memory, since GArray is not designed for holding pointers. This code should probably be refactored further.
*** Bug 774817 has been marked as a duplicate of this bug. ***