Bug 72558 – [Qt] load(), title(), url(), API functions are leaking memory

• Component: WebKit2
• Status: RESOLVED FIXED

[Oleg Romashin (:romaxa)]

QtWebPageProxy::load, QtWebPageProxy::url, QtWebPageProxy::title, and some other API functions leaking strings.

==4060== 98 (36 direct, 62 indirect) bytes in 1 blocks are definitely lost in loss record 2,932 of 3,759
==4060==    at 0x4025018: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==4060==    by 0x5702890: WTF::fastMalloc(unsigned int) (FastMalloc.cpp:268)
==4060==    by 0x472C622: WTF::RefCounted<WebKit::APIObject>::operator new(unsigned int) (RefCounted.h:178)
==4060==    by 0x472E803: WebKit::WebURL::create(WTF::String const&) (WebURL.h:46)
==4060==    by 0x472E916: WebKit::toCopiedURLAPI(WTF::String const&) (WKSharedAPICast.h:162)
==4060==    by 0x4784CB2: WKURLCreateWithQUrl(QUrl const&) (WKURLQt.cpp:34)
==4060==    by 0x484BB7D: QtWebPageProxy::load(QUrl const&) (QtWebPageProxy.cpp:718)
==4060==    by 0x4788D3C: QQuickWebView::load(QUrl const&) (qquickwebview.cpp:390)

WKRetainPtr<WKURLRef> wkurl(WKURLCreateWithQUrl(url));
WKPageLoadURL(pageRef(), wkurl.get());


WebURL created by WKURLCreateWithQUrl, getting refcount + 1 in '
WKURLRef toCopiedURLAPI
RefPtr<WebURL> webURL = WebURL::create(string);
return toAPI(webURL.release().leakRef());

but it never released after WKRetainPtr<WKURLRef> wkurl destroyed...
Not sure what is right way to fix it... I guess on WKRetainPtr<WKURLRef> wkurl destroy it should destroy adopted toImpl WebURL pointer..

Quick workaround to fix this problem is to add:
PassRefPtr<WebURL/WebString> autoDeletePtr = adoptRef(toImpl(wkrefPtr));
which will destroy WebURL/WebString instance on exit from the function...

Otherwise I need to go deeply into smart-pointers relation ship and figure out how to make it works automatically...


Any ideas?

[Sam Weinig]

I believe the issue is this line:

WKRetainPtr<WKURLRef> wkurl(WKURLCreateWithQUrl(url));

that should read:

WKRetainPtr<WKURLRef> wkurl = adoptWK(WKURLCreateWithQUrl(url));

This follow the Create Rule (http://developer.apple.com/library/IOs/#documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html).

[Oleg Romashin (:romaxa)]

> WKRetainPtr<WKURLRef> wkurl = adoptWK(WKURLCreateWithQUrl(url));
> 
Ok thanks Sam, I'll create patch soon

[Oleg Romashin (:romaxa)]

Created attachment 115700 [details]
adoptWK ref pointers in order to release implementation when WK pointer released

[Noam Rosenthal]

Comment on attachment 115700 [details]
adoptWK ref pointers in order to release implementation when WK pointer released

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

Patch is good, please improve the changelog.

> Source/WebKit2/ChangeLog:5
> +

Please explain what the patch actually does.

[Oleg Romashin (:romaxa)]

Created attachment 115848 [details]
use adoptWK in order to adopt implementation refPtr and release it when WK pointer released

[Simon Hausmann]

Comment on attachment 115848 [details]
use adoptWK in order to adopt implementation refPtr and release it when WK pointer released

r=me, but before landing the ChangeLog text should be improved ("don't" -> "doesn't", "adopt" -> "adopts", etc).

[Simon Hausmann]

Landed with updated changelog and rebased against ClientImpl changes

Committed r101136: <http://trac.webkit.org/changeset/101136>