148769 2015-09-03 18:45:57 -0700 Leak in WebContextInjectedBundleClient::getInjectedBundleInitializationUserData 2015-09-05 19:33:47 -0700 1 1 1 Unclassified WebKit WebKit2 Other PC Linux RESOLVED FIXED P2 Minor --- 1 mcatanzaro mcatanzaro andersca commit-queue darin mcatanzaro sam zan oldest_to_newest 1123266 0 mcatanzaro 2015-09-03 18:45:57 -0700 Found by asan. I tried to reason through this but couldn't spot the leak. Beginning in the GTK-specific codepath: API::String::create creates a new String with operator new. Refcount is 1. Passes it into adoptRef. Refcount is still 1. Returns a Ref<String>. WKStringCreateWithUTF8CString leaks the ref, converting it to a String. Refcount is still 1. The String is passed into toAPI to convert it to a WKStringRef. That's just a static_cast, so refcount remains 1. getInjectedBundleInitializationUserData casts it to a WKTypeRef. Refcount remains 1. << end GTK+ specific code >> Meanwhile in the Cocoa code, getInjectedBundleInitializationUserData created an ObjCObjectGraph (refcount 1), leaked the ref (refcount remains 1), and passed it through toAPI. A WKTypeRef is returned. Refcount remains 1. The above are both equivalent. Continuing into cross-platform world: The WKTypeRef (refcount 1) returned by getInjectedBundleInitializationUserData is passed to toImpl, returns as a API::Object (refcount 1). The function returns a PassRefPtr<API::Object>. Since the constructor of the PassRefPtr doesn't modify the refcount, it's still 1. WebProcessPool::createNewWebProcess receives the PassRefPtr<API::Object> and stores it in a RefPtr<API::Object> using operator=. Refcount is still 1. At the end of the function, it will leave scope and be decremented. The raw pointer is retrieved using RefPtr::get, then passed to WebProcessProxy::transformObjectsToHandles. Refcount is still 1. Here we have platform-specific code again. For GTK+ the function is a no-op that returns the input in a RefPtr<API::Object>. In the constructor of the RefPtr, the refcount is incremented, so now it is 2 on GTK+. For Mac, a new ObjCObjectGraph is created, so on Mac there are two separate API::Objects with refcount 1. Still fine. The result is passed to the UserData constructor as a RefPtr<API::Object>&&, which is WTF::move-ed into the UserData's RefPtr<API::Object> (no refcount change... hopefully!). Then it gets copied into the WebProcessCreationParameters, which is declared on the stack and will be destroyed at the end of WebProcessPool::createNewWebProcess. In the olden days the refcount would be increased by 1 (to 3 for GTK+, 2 for Mac), then immediately decremented after the copy when the rvalue is destroyed, but I think nowadays return value optimization avoids that? Doesn't matter; the end result is the refcount is 2 for GTK+, 1 (and 1 on another object) for Mac. At the end of WebProcessPool::createNewWebProcess, it will be decreased from 2 by 2 for GTK+, and by 1 and 1 on two different objects for Mac, as the injectedBundleInitializationUserData RefPtr is destroyed, and as the WebProcessCreationParameters containing the UserData containing the RefPtr<API::Object> is destroyed. So everything works properly. Something in the above reasoning is wrong, because it ain't working properly: Direct leak of 72 byte(s) in 3 object(s) allocated from: #0 0x7fe04e9d4a0a in malloc (/lib64/libasan.so.2+0x98a0a) #1 0x7fe03df78285 in bmalloc::Allocator::allocateSlowCase(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1dd3285) #2 0x7fe03def87ac in bmalloc::Allocator::allocate(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1d537ac) #3 0x7fe03def86e5 in bmalloc::Cache::allocate(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1d536e5) #4 0x7fe03def7928 in bmalloc::api::malloc(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1d52928) #5 0x7fe03def7278 in WTF::fastMalloc(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1d52278) #6 0x7fe044576728 in WTF::ThreadSafeRefCountedBase::operator new(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x534f728) #7 0x7fe0446a600a in API::String::create(WTF::String&&) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x547f00a) #8 0x7fe04472d8ee in WKStringCreateWithUTF8CString (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x55068ee) #9 0x7fe044cdff2b in getInjectedBundleInitializationUserData(OpaqueWKContext const*, void const*) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5ab8f2b) #10 0x7fe0447893b0 in WebKit::WebContextInjectedBundleClient::getInjectedBundleInitializationUserData(WebKit::WebProcessPool*) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x55623b0) #11 0x7fe04486ddd5 in WebKit::WebProcessPool::createNewWebProcess() (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5646dd5) #12 0x7fe04486f375 in WebKit::WebProcessPool::createNewWebProcessRespectingProcessCountLimit() (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5648375) #13 0x7fe04486f6b8 in WebKit::WebProcessPool::createWebPage(WebKit::PageClient&, WTF::Ref<API::PageConfiguration>&&) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x56486b8) #14 0x7fe044d36c08 in webkitWebViewBaseCreateWebPage(_WebKitWebViewBase*, WTF::Ref<API::PageConfiguration>&&) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5b0fc08) #15 0x7fe044d0b161 in webkitWebContextCreatePageForWebView(_WebKitWebContext*, _WebKitWebView*, _WebKitUserContentManager*, _WebKitWebView*) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5ae4161) #16 0x7fe044d29dd6 in webkitWebViewConstructed(_GObject*) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5b02dd6) #17 0x4a5282 in ephy_web_view_constructed /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-web-view.c:928 #18 0x7fe038e6e363 in g_object_new_internal /home/mcatanzaro/jhbuild/checkout/glib/gobject/gobject.c:1820 #19 0x7fe038e6ffc4 in g_object_new_valist /home/mcatanzaro/jhbuild/checkout/glib/gobject/gobject.c:2039 #20 0x7fe038e7012d in g_object_new /home/mcatanzaro/jhbuild/checkout/glib/gobject/gobject.c:1623 #21 0x4a8cd1 in ephy_web_view_new /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-web-view.c:2215 #22 0x4271d1 in ephy_shell_new_tab_full /home/mcatanzaro/jhbuild/checkout/epiphany/src/ephy-shell.c:692 #23 0x4593ea in session_parse_embed /home/mcatanzaro/jhbuild/checkout/epiphany/src/ephy-session.c:1067 #24 0x45957f in session_start_element /home/mcatanzaro/jhbuild/checkout/epiphany/src/ephy-session.c:1113 #25 0x7fe038b706f9 in emit_start_element /home/mcatanzaro/jhbuild/checkout/glib/glib/gmarkup.c:1049 #26 0x7fe038b70ffb in g_markup_parse_context_parse /home/mcatanzaro/jhbuild/checkout/glib/glib/gmarkup.c:1396 #27 0x459de3 in load_stream_read_cb /home/mcatanzaro/jhbuild/checkout/epiphany/src/ephy-session.c:1266 #28 0x7fe03910f4bf in async_ready_callback_wrapper /home/mcatanzaro/jhbuild/checkout/glib/gio/ginputstream.c:529 #29 0x7fe039137d2c in g_task_return_now /home/mcatanzaro/jhbuild/checkout/glib/gio/gtask.c:1104 Indirect leak of 468 byte(s) in 3 object(s) allocated from: #0 0x7fe04e9d4a0a in malloc (/lib64/libasan.so.2+0x98a0a) #1 0x7fe03df78285 in bmalloc::Allocator::allocateSlowCase(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1dd3285) #2 0x7fe03def87ac in bmalloc::Allocator::allocate(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1d537ac) #3 0x7fe03def86e5 in bmalloc::Cache::allocate(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1d536e5) #4 0x7fe03def7928 in bmalloc::api::malloc(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1d52928) #5 0x7fe03def7278 in WTF::fastMalloc(unsigned long) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1d52278) #6 0x7fe03df48f1a in WTF::Ref<WTF::StringImpl> WTF::StringImpl::createUninitializedInternalNonEmpty<unsigned char>(unsigned int, unsigned char*&) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1da3f1a) #7 0x7fe03df48e22 in WTF::Ref<WTF::StringImpl> WTF::StringImpl::createInternal<unsigned char>(unsigned char const*, unsigned int) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1da3e22) #8 0x7fe03df3b18d in WTF::StringImpl::create(unsigned char const*, unsigned int) (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1d9618d) #9 0x7fe03d31acfd in WTF::StringImpl::isolatedCopy() const (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1175cfd) #10 0x7fe03df57d12 in WTF::String::isolatedCopy() const & (/home/mcatanzaro/jhbuild/install/lib/libjavascriptcoregtk-4.0.so.18+0x1db2d12) #11 0x7fe0446a603b in API::String::create(WTF::String&&) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x547f03b) #12 0x7fe04472d8ee in WKStringCreateWithUTF8CString (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x55068ee) #13 0x7fe044cdff2b in getInjectedBundleInitializationUserData(OpaqueWKContext const*, void const*) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5ab8f2b) #14 0x7fe0447893b0 in WebKit::WebContextInjectedBundleClient::getInjectedBundleInitializationUserData(WebKit::WebProcessPool*) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x55623b0) #15 0x7fe04486ddd5 in WebKit::WebProcessPool::createNewWebProcess() (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5646dd5) #16 0x7fe04486f375 in WebKit::WebProcessPool::createNewWebProcessRespectingProcessCountLimit() (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5648375) #17 0x7fe04486f6b8 in WebKit::WebProcessPool::createWebPage(WebKit::PageClient&, WTF::Ref<API::PageConfiguration>&&) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x56486b8) #18 0x7fe044d36c08 in webkitWebViewBaseCreateWebPage(_WebKitWebViewBase*, WTF::Ref<API::PageConfiguration>&&) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5b0fc08) #19 0x7fe044d0b161 in webkitWebContextCreatePageForWebView(_WebKitWebContext*, _WebKitWebView*, _WebKitUserContentManager*, _WebKitWebView*) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5ae4161) #20 0x7fe044d29dd6 in webkitWebViewConstructed(_GObject*) (/home/mcatanzaro/jhbuild/install/lib/libwebkit2gtk-4.0.so.37+0x5b02dd6) #21 0x4a5282 in ephy_web_view_constructed /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-web-view.c:928 #22 0x7fe038e6e363 in g_object_new_internal /home/mcatanzaro/jhbuild/checkout/glib/gobject/gobject.c:1820 #23 0x7fe038e6ffc4 in g_object_new_valist /home/mcatanzaro/jhbuild/checkout/glib/gobject/gobject.c:2039 #24 0x7fe038e7012d in g_object_new /home/mcatanzaro/jhbuild/checkout/glib/gobject/gobject.c:1623 #25 0x4a8cd1 in ephy_web_view_new /home/mcatanzaro/jhbuild/checkout/epiphany/embed/ephy-web-view.c:2215 #26 0x4271d1 in ephy_shell_new_tab_full /home/mcatanzaro/jhbuild/checkout/epiphany/src/ephy-shell.c:692 #27 0x4593ea in session_parse_embed /home/mcatanzaro/jhbuild/checkout/epiphany/src/ephy-session.c:1067 #28 0x45957f in session_start_element /home/mcatanzaro/jhbuild/checkout/epiphany/src/ephy-session.c:1113 #29 0x7fe038b706f9 in emit_start_element /home/mcatanzaro/jhbuild/checkout/glib/glib/gmarkup.c:1049 1123915 1 zan 2015-09-05 01:32:49 -0700 (In reply to comment #0) > > Meanwhile in the Cocoa code, getInjectedBundleInitializationUserData created > an ObjCObjectGraph (refcount 1), leaked the ref (refcount remains 1), and > passed it through toAPI. A WKTypeRef is returned. Refcount remains 1. The > above are both equivalent. Continuing into cross-platform world: > > The WKTypeRef (refcount 1) returned by > getInjectedBundleInitializationUserData is passed to toImpl, returns as a > API::Object (refcount 1). The function returns a PassRefPtr<API::Object>. > Since the constructor of the PassRefPtr doesn't modify the refcount, it's > still 1. > The pointer isn't adopted, so the PassRefPtr constructor does increment the refcount to 2. > WebProcessPool::createNewWebProcess receives the PassRefPtr<API::Object> and > stores it in a RefPtr<API::Object> using operator=. Refcount is still 1. At > the end of the function, it will leave scope and be decremented. > The PassRefPtr object is moved into the RefPtr, which doesn't change the refcount of the wrapped object -- still 2. Adopting the pointer into the PassRefPtr in WebContextInjectedBundleClient::getInjectedBundleInitializationUserData() would probably fix the leak. 1123929 2 mcatanzaro 2015-09-05 07:14:15 -0700 (In reply to comment #1) > Adopting the pointer into the PassRefPtr in > WebContextInjectedBundleClient::getInjectedBundleInitializationUserData() > would probably fix the leak. Yes, this fixes it: - return toImpl(m_client.getInjectedBundleInitializationUserData(toAPI(processPool), m_client.base.clientInfo)); + return adoptRef(toImpl(m_client.getInjectedBundleInitializationUserData(toAPI(processPool), m_client.base.clientInfo)); A clue was that the result of toImpl was adopted one function up. (Feel free to submit the patch if you want, since you found it; otherwise I will.) 1123937 3 zan 2015-09-05 09:17:41 -0700 Go ahead. 1123969 4 mcatanzaro 2015-09-05 13:22:45 -0700 CCing Darin since we need an owner 1123970 5 260698 mcatanzaro 2015-09-05 13:23:16 -0700 Created attachment 260698 Patch 1123990 6 260698 commit-queue 2015-09-05 19:33:43 -0700 Comment on attachment 260698 Patch Clearing flags on attachment: 260698 Committed r189442: <http://trac.webkit.org/changeset/189442> 1123991 7 commit-queue 2015-09-05 19:33:47 -0700 All reviewed patches have been landed. Closing bug. 260698 2015-09-05 13:23:16 -0700 2015-09-05 19:33:43 -0700 Patch bug-148769-20150905152313.patch text/plain 1657 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMTg5MTgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViS2l0Mi9DaGFuZ2VMb2cKaW5kZXggNDc4ZTU3NWQ4OWY4NzMy YjY1YjQ3YzdiM2I1YTM4NzVjY2Y5Nzk3OS4uMGZlN2ExYTc5ZTNhYzNjMzcxZGUxNzk0YjVmMDhm MmNlMmE2NzA4NyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJLaXQyL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDE1LTA5LTA1ICBNaWNo YWVsIENhdGFuemFybyAgPG1jYXRhbnphcm9AaWdhbGlhLmNvbT4KKworICAgICAgICBMZWFrIGlu IFdlYkNvbnRleHRJbmplY3RlZEJ1bmRsZUNsaWVudDo6Z2V0SW5qZWN0ZWRCdW5kbGVJbml0aWFs aXphdGlvblVzZXJEYXRhCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVn LmNnaT9pZD0xNDg3NjkKKworICAgICAgICBGaXggc3VnZ2VzdGVkIGJ5IFphbiBEb2JlcnNlay4K KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVJUHJv Y2Vzcy9XZWJDb250ZXh0SW5qZWN0ZWRCdW5kbGVDbGllbnQuY3BwOgorICAgICAgICAoV2ViS2l0 OjpXZWJDb250ZXh0SW5qZWN0ZWRCdW5kbGVDbGllbnQ6OmdldEluamVjdGVkQnVuZGxlSW5pdGlh bGl6YXRpb25Vc2VyRGF0YSk6CisKIDIwMTUtMDgtMzEgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hv cnRvbkBhcHBsZS5jb20+CiAKICAgICAgICAgaU9TIFdlYktpdDIgZmluZC1pbi1wYWdlIGRvZXNu J3Qgc3VwcG9ydCBtdWx0aS1saW5lIHJlc3VsdHMsIGlzIG9mdGVuIGJsYW5rCmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViQ29udGV4dEluamVjdGVkQnVuZGxlQ2xpZW50 LmNwcCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJDb250ZXh0SW5qZWN0ZWRCdW5kbGVD bGllbnQuY3BwCmluZGV4IDFkMTU1NWEwOWM2Mzg1ZDMyMjA1MzIwOTkyOGUwM2U0NTZjMzhkZWUu LmM2OGExNjNlZTVmOGI3NWY1NGZhYWE4NWNiNmEwZjk2YmUyY2UxNmYgMTAwNjQ0Ci0tLSBhL1Nv dXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJDb250ZXh0SW5qZWN0ZWRCdW5kbGVDbGllbnQuY3Bw CisrKyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJDb250ZXh0SW5qZWN0ZWRCdW5kbGVD bGllbnQuY3BwCkBAIC01Nyw3ICs1Nyw3IEBAIFBhc3NSZWZQdHI8QVBJOjpPYmplY3Q+IFdlYkNv bnRleHRJbmplY3RlZEJ1bmRsZUNsaWVudDo6Z2V0SW5qZWN0ZWRCdW5kbGVJbml0aWFsCiAgICAg aWYgKCFtX2NsaWVudC5nZXRJbmplY3RlZEJ1bmRsZUluaXRpYWxpemF0aW9uVXNlckRhdGEpCiAg ICAgICAgIHJldHVybiAwOwogCi0gICAgcmV0dXJuIHRvSW1wbChtX2NsaWVudC5nZXRJbmplY3Rl ZEJ1bmRsZUluaXRpYWxpemF0aW9uVXNlckRhdGEodG9BUEkocHJvY2Vzc1Bvb2wpLCBtX2NsaWVu dC5iYXNlLmNsaWVudEluZm8pKTsKKyAgICByZXR1cm4gYWRvcHRSZWYodG9JbXBsKG1fY2xpZW50 LmdldEluamVjdGVkQnVuZGxlSW5pdGlhbGl6YXRpb25Vc2VyRGF0YSh0b0FQSShwcm9jZXNzUG9v bCksIG1fY2xpZW50LmJhc2UuY2xpZW50SW5mbykpKTsKIH0KIAogfSAvLyBuYW1lc3BhY2UgV2Vi S2l0Cg==