Bug 148769 – Leak in WebContextInjectedBundleClient::getInjectedBundleInitializationUserData

• Component: WebKit2
• Status: RESOLVED FIXED

[Michael Catanzaro]

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

[Zan Dobersek]

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

[Michael Catanzaro]

(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.)

[Zan Dobersek]

Go ahead.

[Michael Catanzaro]

CCing Darin since we need an owner

[Michael Catanzaro]

Created attachment 260698 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 260698 [details]
Patch

Clearing flags on attachment: 260698

Committed r189442: <http://trac.webkit.org/changeset/189442>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.