Bug 70505 – [GTK] [WK2] Memory leaks in WebContextGtk.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Sudarsana Nagineni (babu)]

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.

[Sudarsana Nagineni (babu)]

Created attachment 111792 [details]
Fix memory leaks

Free the output of g_build_filename()

[Martin Robinson]

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.

[Sudarsana Nagineni (babu)]

(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!

[Sudarsana Nagineni (babu)]

Created attachment 112123 [details]
Patch

Fixed review comment

[WebKit Review Bot]

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.

[Sudarsana Nagineni (babu)]

Created attachment 112124 [details]
Patch

Fixed review comment + Style

[Martin Robinson]

Thanks!

[WebKit Review Bot]

Comment on attachment 112124 [details]
Patch

Clearing flags on attachment: 112124

Committed r98224: <http://trac.webkit.org/changeset/98224>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.