Bug 70505 - [GTK] [WK2] Memory leaks in WebContextGtk.cpp
Summary: [GTK] [WK2] Memory leaks in WebContextGtk.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sudarsana Nagineni (babu)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-20 08:41 PDT by Sudarsana Nagineni (babu)
Modified: 2012-05-15 00:20 PDT (History)
2 users (show)

See Also:


Attachments
Fix memory leaks (2.02 KB, patch)
2011-10-20 09:25 PDT, Sudarsana Nagineni (babu)
mrobinson: review-
Details | Formatted Diff | Diff
Patch (1.99 KB, patch)
2011-10-23 12:37 PDT, Sudarsana Nagineni (babu)
no flags Details | Formatted Diff | Diff
Patch (2.07 KB, patch)
2011-10-23 12:57 PDT, Sudarsana Nagineni (babu)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sudarsana Nagineni (babu) 2011-10-20 08:41:44 PDT
Steps To Reproduce: 
1) Build GTK WEBKIT2: 
Tools/Scripts/build-webkit --gtk --debug --enable-webkit2
2) Run MinBrowser with valgrind:
valgrind --leak-check=yes --trace-children=yes --num-callers=50 --smc-check=all --log-file=/tmp/valgrind.log ./MiniBrowser


==1343== 64 bytes in 1 blocks are definitely lost in loss record 4,318 of 6,592
==1343==    at 0x4C29097: realloc (vg_replace_malloc.c:525)
==1343==    by 0x97FB750: g_realloc (gmem.c:233)
==1343==    by 0x9814BC6: g_string_maybe_expand (gstring.c:401)
==1343==    by 0x9815286: g_string_insert_len (gstring.c:741)
==1343==    by 0x97E0106: g_build_path_va (gfileutils.c:1637)
==1343==    by 0x97E18B7: g_build_filename (gfileutils.c:1907)
==1343==    by 0x58AEB32: WebKit::WebContext::platformDefaultDatabaseDirectory() const (WebContextGtk.cpp:51)
==1343==    by 0x58D1F57: WebKit::WebContext::databaseDirectory() const (WebContext.cpp:780)
==1343==    by 0x58CF7E1: WebKit::WebContext::ensureWebProcess() (WebContext.cpp:245)
==1343==    by 0x58D0179: WebKit::WebContext::createWebPage(WebKit::PageClient*, WebKit::WebPageGroup*) (WebContext.cpp:371)
==1343==    by 0x58A406E: webkitWebViewBaseCreateWebPage (WebKitWebViewBase.cpp:440)
==1343==    by 0x58A3FC9: webkitWebViewBaseCreate (WebKitWebViewBase.cpp:422)
==1343==    by 0x5893D2E: WKViewCreate (WKView.cpp:39)
==1343==    by 0x4053D4: loadURI (main.c:61)
==1343==    by 0x4055C7: main (main.c:109)
==1343==
==1343== 64 bytes in 1 blocks are definitely lost in loss record 4,319 of 6,592
==1343==    at 0x4C29097: realloc (vg_replace_malloc.c:525)
==1343==    by 0x97FB750: g_realloc (gmem.c:233)
==1343==    by 0x9814BC6: g_string_maybe_expand (gstring.c:401)
==1343==    by 0x9815286: g_string_insert_len (gstring.c:741)
==1343==    by 0x97E0106: g_build_path_va (gfileutils.c:1637)
==1343==    by 0x97E18B7: g_build_filename (gfileutils.c:1907)
==1343==    by 0x58AEBA2: WebKit::WebContext::platformDefaultLocalStorageDirectory() const (WebContextGtk.cpp:66)
==1343==    by 0x58D2061: WebKit::WebContext::localStorageDirectory() const (WebContext.cpp:802)
==1343==    by 0x58CF81D: WebKit::WebContext::ensureWebProcess() (WebContext.cpp:246)
==1343==    by 0x58D0179: WebKit::WebContext::createWebPage(WebKit::PageClient*, WebKit::WebPageGroup*) (WebContext.cpp:371)
==1343==    by 0x58A406E: webkitWebViewBaseCreateWebPage (WebKitWebViewBase.cpp:440)
==1343==    by 0x58A3FC9: webkitWebViewBaseCreate (WebKitWebViewBase.cpp:422)
==1343==    by 0x5893D2E: WKViewCreate (WKView.cpp:39)
==1343==    by 0x4053D4: loadURI (main.c:61)
==1343==    by 0x4055C7: main (main.c:109)

Newly allocated string by g_build_filename() must be freed to avoid these leaks.
Comment 1 Sudarsana Nagineni (babu) 2011-10-20 09:25:16 PDT
Created attachment 111792 [details]
Fix memory leaks

Free the output of g_build_filename()
Comment 2 Martin Robinson 2011-10-21 13:23:55 PDT
Comment on attachment 111792 [details]
Fix memory leaks

View in context: https://bugs.webkit.org/attachment.cgi?id=111792&action=review

Nice catch! Just a small suggestion below.

> Source/WebKit2/UIProcess/gtk/WebContextGtk.cpp:51
> +    return WTF::String::fromUTF8(databaseDirectory.get());

I know this was still an issue before, but do you mind using filenameToString from FileSystemGtk instead of fromUTF8 here? It's more correct.

> Source/WebKit2/UIProcess/gtk/WebContextGtk.cpp:63
> +    return WTF::String::fromUTF8(storageDirectory.get());

Ditto.
Comment 3 Sudarsana Nagineni (babu) 2011-10-23 09:46:19 PDT
(In reply to comment #2)
> > Source/WebKit2/UIProcess/gtk/WebContextGtk.cpp:51
> > +    return WTF::String::fromUTF8(databaseDirectory.get());
> 
> I know this was still an issue before, but do you mind using filenameToString from FileSystemGtk instead of fromUTF8 here? It's more correct.

Okay, I will change this. Thanks for reviewing Martin!
Comment 4 Sudarsana Nagineni (babu) 2011-10-23 12:37:53 PDT
Created attachment 112123 [details]
Patch

Fixed review comment
Comment 5 WebKit Review Bot 2011-10-23 12:39:37 PDT
Attachment 112123 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/gtk/WebContextGtk.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Sudarsana Nagineni (babu) 2011-10-23 12:57:36 PDT
Created attachment 112124 [details]
Patch

Fixed review comment + Style
Comment 7 Martin Robinson 2011-10-24 00:49:37 PDT
Thanks!
Comment 8 WebKit Review Bot 2011-10-24 01:53:31 PDT
Comment on attachment 112124 [details]
Patch

Clearing flags on attachment: 112124

Committed r98224: <http://trac.webkit.org/changeset/98224>
Comment 9 WebKit Review Bot 2011-10-24 01:53:36 PDT
All reviewed patches have been landed.  Closing bug.