After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 696484 - Memory leak in URITester compile regexp method
Memory leak in URITester compile regexp method
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-03-24 08:23 UTC by Xan Lopez
Modified: 2015-09-08 17:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
uri-tester: Create optslist HashTable with a free function fo the keys (1.13 KB, patch)
2015-08-31 07:15 UTC, Carlos Garcia Campos
committed Details | Review
uri-tester: don't leak from the pattern table (935 bytes, patch)
2015-08-31 14:44 UTC, Michael Catanzaro
committed Details | Review

Description Xan Lopez 2013-03-24 08:23:26 UTC
==11764== 21,953 bytes in 691 blocks are definitely lost in loss record 19,587 of 19,718
==11764==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==11764==    by 0xBA2A75E: g_malloc (gmem.c:159)
==11764==    by 0xBA2AA89: g_malloc_n (gmem.c:400)
==11764==    by 0xBA45A1D: g_strdup (gstrfuncs.c:364)
==11764==    by 0x187FF4B1: uri_tester_compile_regexp (uri-tester.c:511)
==11764==    by 0x187FF7C8: uri_tester_add_url_pattern (uri-tester.c:586)
==11764==    by 0x187FFC57: uri_tester_parse_line (uri-tester.c:702)
==11764==    by 0x187FFD2D: uri_tester_parse_file_at_uri (uri-tester.c:724)
==11764==    by 0x187FE6B9: uri_tester_retrieve_filter_finished (uri-tester.c:122)
==11764==    by 0xB86828F: g_task_return_now (gtask.c:1105)
==11764==    by 0xB8682C0: complete_in_idle_cb (gtask.c:1114)
==11764==    by 0xBA24B6A: g_idle_dispatch (gmain.c:5205)
==11764==    by 0xBA223E8: g_main_dispatch (gmain.c:3054)
==11764==    by 0xBA2314D: g_main_context_dispatch (gmain.c:3630)
==11764==    by 0xBA2333D: g_main_context_iterate (gmain.c:3701)
==11764==    by 0xBA2376D: g_main_loop_run (gmain.c:3895)
==11764==    by 0x79BFB2D: WebCore::RunLoop::run() (RunLoopGtk.cpp:61)
==11764==    by 0x622EE70: WebProcessMainGtk (WebProcessMainGtk.cpp:83)
==11764==    by 0x400C2B: main (MainGtk.cpp:31)
Comment 1 Michael Catanzaro 2015-08-31 02:08:42 UTC
Still here. Both strings are regex are being leaked, from at least tester->priv->pattern and tester->priv->optslist, but maybe other HashTables too. The regex are allocated on line 483 of course. I won't copy all the various instances of leaked strings, but they're allocated in many different places: lines 505, 514, 515, and 530, enough places to be confident they're leaked from basically every HashTable.

Indirect leak of 29 byte(s) in 1 object(s) allocated from:
    #0 0x49c7cb in malloc (/home/mcatanzaro/jhbuild/install/libexec/webkit2gtk-4.0/WebKitWebProcess+0x49c7cb)
    #1 0x7f08f7d06068 in g_malloc /home/mcatanzaro/jhbuild/checkout/glib/glib/gmem.c:94
    #2 0x7f08f7d0634a in g_malloc_n /home/mcatanzaro/jhbuild/checkout/glib/glib/gmem.c:330
    #3 0x7f08f7d21077 in g_strdup /home/mcatanzaro/jhbuild/checkout/glib/glib/gstrfuncs.c:363
    #4 0x7f08f7d13e0c in g_regex_new /home/mcatanzaro/jhbuild/checkout/glib/glib/gregex.c:1346
    #5 0x7f089772f2ec in uri_tester_compile_regexp /home/mcatanzaro/jhbuild/checkout/epiphany/embed/web-extension/../../embed/uri-tester.c:483
    #6 0x7f089772fed5 in uri_tester_add_url_pattern /home/mcatanzaro/jhbuild/checkout/epiphany/embed/web-extension/../../embed/uri-tester.c:581
    #7 0x7f08977309b2 in uri_tester_parse_line /home/mcatanzaro/jhbuild/checkout/epiphany/embed/web-extension/../../embed/uri-tester.c:705
    #8 0x7f0897730b42 in file_parse_cb /home/mcatanzaro/jhbuild/checkout/epiphany/embed/web-extension/../../embed/uri-tester.c:727
    #9 0x7f08f850a911 in g_task_return_now /home/mcatanzaro/jhbuild/checkout/glib/gio/gtask.c:1104
    #10 0x7f08f850a96e in complete_in_idle_cb /home/mcatanzaro/jhbuild/checkout/glib/gio/gtask.c:1118
    #11 0x7f08f7d00430 in g_idle_dispatch /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:5441
    #12 0x7f08f7cfda78 in g_main_dispatch /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:3154
    #13 0x7f08f7cfe8bc in g_main_context_dispatch /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:3769
    #14 0x7f08f7cfeaa0 in g_main_context_iterate /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:3840
    #15 0x7f08f7cfeec6 in g_main_loop_run /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:4034
    #16 0x7f09059b2a68 in WTF::RunLoop::run() /home/mcatanzaro/WebKit/WebKitBuild/GNOME/../../Source/WTF/wtf/glib/RunLoopGLib.cpp:67:9
    #17 0x7f09033c28a0 in int WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain>(int, char**) /home/mcatanzaro/WebKit/WebKitBuild/GNOME/../../Source/WebKit2/Shared/unix/ChildProcessMain.h:61:5
    #18 0x7f09033c2778 in WebProcessMainUnix /home/mcatanzaro/WebKit/WebKitBuild/GNOME/../../Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:77:12
    #19 0x4b9f76 in main /home/mcatanzaro/WebKit/WebKitBuild/GNOME/../../Source/WebKit2/WebProcess/EntryPoint/unix/WebProcessMain.cpp:44:12
    #20 0x7f08f15cd6ff in __libc_start_main (/lib64/libc.so.6+0x206ff)


Direct leak of 29 byte(s) in 1 object(s) allocated from:
    #0 0x49c7cb in malloc (/home/mcatanzaro/jhbuild/install/libexec/webkit2gtk-4.0/WebKitWebProcess+0x49c7cb)
    #1 0x7f08f7d06068 in g_malloc /home/mcatanzaro/jhbuild/checkout/glib/glib/gmem.c:94
    #2 0x7f08f7d0634a in g_malloc_n /home/mcatanzaro/jhbuild/checkout/glib/glib/gmem.c:330
    #3 0x7f08f7d21077 in g_strdup /home/mcatanzaro/jhbuild/checkout/glib/glib/gstrfuncs.c:363
    #4 0x7f089772f716 in uri_tester_compile_regexp /home/mcatanzaro/jhbuild/checkout/epiphany/embed/web-extension/../../embed/uri-tester.c:514
    #5 0x7f089772fed5 in uri_tester_add_url_pattern /home/mcatanzaro/jhbuild/checkout/epiphany/embed/web-extension/../../embed/uri-tester.c:581
    #6 0x7f08977309b2 in uri_tester_parse_line /home/mcatanzaro/jhbuild/checkout/epiphany/embed/web-extension/../../embed/uri-tester.c:705
    #7 0x7f0897730b42 in file_parse_cb /home/mcatanzaro/jhbuild/checkout/epiphany/embed/web-extension/../../embed/uri-tester.c:727
    #8 0x7f08f850a911 in g_task_return_now /home/mcatanzaro/jhbuild/checkout/glib/gio/gtask.c:1104
    #9 0x7f08f850a96e in complete_in_idle_cb /home/mcatanzaro/jhbuild/checkout/glib/gio/gtask.c:1118
    #10 0x7f08f7d00430 in g_idle_dispatch /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:5441
    #11 0x7f08f7cfda78 in g_main_dispatch /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:3154
    #12 0x7f08f7cfe8bc in g_main_context_dispatch /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:3769
    #13 0x7f08f7cfeaa0 in g_main_context_iterate /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:3840
    #14 0x7f08f7cfeec6 in g_main_loop_run /home/mcatanzaro/jhbuild/checkout/glib/glib/gmain.c:4034
    #15 0x7f09059b2a68 in WTF::RunLoop::run() /home/mcatanzaro/WebKit/WebKitBuild/GNOME/../../Source/WTF/wtf/glib/RunLoopGLib.cpp:67:9
    #16 0x7f09033c28a0 in int WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain>(int, char**) /home/mcatanzaro/WebKit/WebKitBuild/GNOME/../../Source/WebKit2/Shared/unix/ChildProcessMain.h:61:5
    #17 0x7f09033c2778 in WebProcessMainUnix /home/mcatanzaro/WebKit/WebKitBuild/GNOME/../../Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:77:12
    #18 0x4b9f76 in main /home/mcatanzaro/WebKit/WebKitBuild/GNOME/../../Source/WebKit2/WebProcess/EntryPoint/unix/WebProcessMain.cpp:44:12
    #19 0x7f08f15cd6ff in __libc_start_main (/lib64/libc.so.6+0x206ff)
Comment 2 Carlos Garcia Campos 2015-08-31 07:15:56 UTC
Created attachment 310326 [details] [review]
uri-tester: Create optslist HashTable with a free function fo the keys

I haven't looked at it in detail, but at a first glance I'm pretty sure they keys of the optslist hashtable are leaked, since they are added with g_strdup, but never freed because the has table is created with a NULL free function for the keys. I guess there are more leaks, though, there's a suspicious g_hash_table_steal I don't understand.
Comment 3 Michael Catanzaro 2015-08-31 14:42:05 UTC
Ah, it seems obvious, but I missed that. I did think the steal was wrong, but it didn't explain the problem with optslist.

When I change the steal to a remove, the adblocker still works and the complaints seem to go away, in combination with your patch.
Comment 4 Michael Catanzaro 2015-08-31 14:44:35 UTC
Created attachment 310368 [details] [review]
uri-tester: don't leak from the pattern table

This code is difficult to understand, but it's clear that these elements
are not freed when they are removed. Fix it.
Comment 5 Michael Catanzaro 2015-08-31 14:48:22 UTC
The other interesting thing to note is that if you insert an element that's already present in the HashTable, it will be freed automatically by the GDestroyNotify.
Comment 6 Michael Catanzaro 2015-09-08 17:00:10 UTC
Again, lgtm from Claudio
Comment 7 Michael Catanzaro 2015-09-08 17:40:19 UTC
Attachment 310326 [details] pushed as f324749 - uri-tester: Create optslist HashTable with a free function fo the keys
Attachment 310368 [details] pushed as aa9fb6b - uri-tester: don't leak from the pattern table