RESOLVED FIXED 72558
[Qt] load(), title(), url(), API functions are leaking memory
https://bugs.webkit.org/show_bug.cgi?id=72558
Summary [Qt] load(), title(), url(), API functions are leaking memory
Oleg Romashin (:romaxa)
Reported 2011-11-16 15:57:36 PST
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?
Attachments
adoptWK ref pointers in order to release implementation when WK pointer released (3.57 KB, patch)
2011-11-17 15:31 PST, Oleg Romashin (:romaxa)
noam: review-
use adoptWK in order to adopt implementation refPtr and release it when WK pointer released (3.82 KB, patch)
2011-11-18 12:34 PST, Oleg Romashin (:romaxa)
hausmann: review+
hausmann: commit-queue-
Sam Weinig
Comment 1 2011-11-16 16:21:12 PST
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)
Comment 2 2011-11-17 09:29:00 PST
> WKRetainPtr<WKURLRef> wkurl = adoptWK(WKURLCreateWithQUrl(url)); > Ok thanks Sam, I'll create patch soon
Oleg Romashin (:romaxa)
Comment 3 2011-11-17 15:31:28 PST
Created attachment 115700 [details] adoptWK ref pointers in order to release implementation when WK pointer released
Noam Rosenthal
Comment 4 2011-11-17 16:59:07 PST
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)
Comment 5 2011-11-18 12:34:40 PST
Created attachment 115848 [details] use adoptWK in order to adopt implementation refPtr and release it when WK pointer released
Simon Hausmann
Comment 6 2011-11-24 04:15:39 PST
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
Comment 7 2011-11-24 05:45:57 PST
Landed with updated changelog and rebased against ClientImpl changes Committed r101136: <http://trac.webkit.org/changeset/101136>
Note You need to log in before you can comment on or make changes to this bug.