Bug 106137 – REGRESSION(r138222): WebDocumentLoaderMac-related leaks seen on Leaks bot

• Component: Page Loading
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

WebDocumentLoaderMac-related leaks seen on Leaks bot

The leaks bot is producing gigibytes of logs (literally) to document (primarily a set of leaks around WebDocumentLoaderMac

Such as:

        Call stack: [thread 0x7fff734a4180]: | start | main DumpRenderTree.mm:931 | dumpRenderTree(int, char const**) DumpRenderTree.mm:893 | runTestingServerLoop() DumpRenderTree.mm:846 | runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) DumpRenderTree.mm:1417 | -[WebFrame loadHTMLString:baseURL:] WebFrame.mm:1443 | -[WebFrame _loadHTMLString:baseURL:unreachableURL:] WebFrame.mm:1436 | -[WebFrame _loadData:MIMEType:textEncodingName:baseURL:unreachableURL:] WebFrame.mm:1419 | WebCore::FrameLoader::load(WebCore::FrameLoadRequest const&) FrameLoader.cpp:1286 | WebFrameLoaderClient::createDocumentLoader(WebCore::ResourceRequest const&, WebCore::SubstituteData const&) WebFrameLoaderClient.mm:1135 | WebDocumentLoaderMac::create(WebCore::ResourceRequest const&, WebCore::SubstituteData const&) WebDocumentLoaderMac.h:44 | WebDocumentLoaderMac::WebDocumentLoaderMac(WebCore::ResourceRequest const&, WebCore::SubstituteData const&) WebDocumentLoaderMac.mm:41 | WebDocumentLoaderMac::WebDocumentLoaderMac(WebCore::ResourceRequest const&, WebCore::SubstituteData const&) WebDocumentLoaderMac.mm:40 | WebCore::DocumentLoader::DocumentLoader(WebCore::ResourceRequest const&, WebCore::SubstituteData const&) DocumentLoader.cpp:91 | WebCore::ResourceResponse::ResourceResponse() ResourceResponse.h:45 | WebCore::ResourceResponse::ResourceResponse() ResourceResponse.h:44 | WebCore::ResourceResponseBase::ResourceResponseBase() ResourceResponseBase.cpp:70 | WebCore::HTTPHeaderMap::HTTPHeaderMap() HTTPHeaderMap.cpp:42 | WebCore::HTTPHeaderMap::HTTPHeaderMap() HTTPHeaderMap.cpp:42 | WTF::HashMap<WTF::AtomicString, WTF::String, WTF::CaseFoldingHash, WTF::HashTraits<WTF::AtomicString>, WTF::HashTraits<WTF::String> >::HashMap() HashMap.h:43 | WTF::HashTable<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::String>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::String> >, WTF::CaseFoldingHash, WTF::HashMapValueTraits<WTF::HashTraits<WTF::AtomicString>, WTF::HashTraits<WTF::String> >, WTF::HashTraits<WTF::AtomicString> >::HashTable() HashTable.h:560 | WTF::HashTable<WTF::AtomicString, WTF::KeyValuePair<WTF::AtomicString, WTF::String>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::AtomicString, WTF::String> >, WTF::CaseFoldingHash, WTF::HashMapValueTraits<WTF::HashTraits<WTF::AtomicString>, WTF::HashTraits<WTF::String> >, WTF::HashTraits<WTF::AtomicString> >::HashTable() HashTable.h:554 | WTF::Mutex::operator new(unsigned long) ThreadingPrimitives.h:76 | WTF::fastMalloc(unsigned long) FastMalloc.cpp:274 | malloc | malloc_zone_malloc

And:

        Call stack: [thread 0x7fff734a4180]: | start | main DumpRenderTree.mm:931 | dumpRenderTree(int, char const**) DumpRenderTree.mm:893 | runTestingServerLoop() DumpRenderTree.mm:846 | runTest(std::__1::basic_string&lt;char, std::__1::char_traits&lt;char&gt;, std::__1::allocator&lt;char&gt; &gt; const&amp;) DumpRenderTree.mm:1417 | -[WebFrame loadHTMLString:baseURL:] WebFrame.mm:1443 | -[WebFrame _loadHTMLString:baseURL:unreachableURL:] WebFrame.mm:1436 | -[WebFrame _loadData:MIMEType:textEncodingName:baseURL:unreachableURL:] WebFrame.mm:1419 | WebCore::FrameLoader::load(WebCore::FrameLoadRequest const&amp;) FrameLoader.cpp:1286 | WebFrameLoaderClient::createDocumentLoader(WebCore::ResourceRequest const&amp;, WebCore::SubstituteData const&amp;) WebFrameLoaderClient.mm:1135 | WebDocumentLoaderMac::create(WebCore::ResourceRequest const&amp;, WebCore::SubstituteData const&amp;) WebDocumentLoaderMac.h:44 | WTF::RefCounted&lt;WebCore::DocumentLoader&gt;::operator new(unsigned long) RefCounted.h:197 | WTF::fastMalloc(unsigned long) FastMalloc.cpp:274 | malloc | malloc_zone_malloc 
Leak: 0x7fb71886b200  size=2560  zone: DefaultMallocZone_0x10806e000   WebDocumentLoaderMac  C++  WebKit
        0x08388b00 0x00000001 0x00000001 0x01000000     ..8.............
        0x00000000 0x00000000 0x22011c70 0x00007fb7     ........p..&#34;....
        0x00000000 0x00000000 0x00000000 0x00000000     ................
        0x00000000 0x00000000 0x00000000 0x00000000     ................
        0x00000000 0x00000000 0x22011de0 0x00007fb7     ...........&#34;....
        0x00000000 0x00000000 0x00000000 0x00000000     ................
        0x00000000 0x00000000 0x00000000 0x00000000     ................
        0x22011e20 0x00007fb7 0x00000000 0x00000000      ..&#34;............
        ...


It's unclear what exactly is going on, but presumably some major leaking.

[Eric Seidel (no email)]

It's possible we're leaking the DocumentLoader on all ports, but that seems unlikely.

[Eric Seidel (no email)]

The log is here:
http://build.webkit.org/builders/Apple%20MountainLion%20%28Leaks%29/builds/2277/steps/layout-test/logs/stdio

I recommend using curl to grab the first 30mb or so, as its easier to view locally than in a browser. :)

[Adam Barth]

Which test?  All of them?

[Ryosuke Niwa]

(In reply to comment #3)
> Which test?  All of them?

It appears to be happening everywhere.

[Ryosuke Niwa]

Now,
http://trac.webkit.org/changeset/138218: blackberry change
http://trac.webkit.org/changeset/138219: rebaseline
http://trac.webkit.org/changeset/138220: chromium change
http://trac.webkit.org/changeset/138221: ditto
http://trac.webkit.org/changeset/138222: loader change

[Ryosuke Niwa]

Created attachment 181670 [details]
Leak output sample

[Nate Chapin]

(In reply to comment #6)
> Created an attachment (id=181670) [details]
> Leak output sample

I'm fairly confident that this has the same route cause as https://bugs.webkit.org/show_bug.cgi?id=106123

[Nate Chapin]

Created attachment 181805 [details]
patch + test

[Eric Seidel (no email)]

Comment on attachment 181805 [details]
patch + test

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

> Source/WebCore/ChangeLog:14
> +        (WebCore::MainResourceLoader::receivedError): Call dispatchDidFailLoading() if
> +            a SubstituteData load fails or is cancelled. Without this call, load counts
> +            are not balanced on WebDocumentLoaderMac and it is retained forever.

Is there a time in the code where we expect WebDocumentLoaderMac to go away that we can ASSERT it has 1 ref before we drop our last one?

[Alexey Proskuryakov]

Comment on attachment 181805 [details]
patch + test

Brady should probably review this.

Additional state variables in loader make me unhappy though, especially ones as unclear as "m_finishing".

[Nate Chapin]

Created attachment 181942 [details]
Without the class variable

[Brady Eidson]

Comment on attachment 181942 [details]
Without the class variable

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

> Source/WebCore/loader/MainResourceLoader.cpp:103
> +    if (m_substituteDataLoadIdentifier)
> +        frameLoader()->client()->dispatchDidFailLoading(documentLoader(), m_substituteDataLoadIdentifier, error);

Would like to see ASSERT(!loader()) in here.

[Nate Chapin]

Created attachment 182172 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 182172 [details]
Patch for landing

Clearing flags on attachment: 182172

Committed r139343: <http://trac.webkit.org/changeset/139343>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Hot damn!  It will be very interesting to see what the leaks bot shows now that this huge leak is gone.

[Ryosuke Niwa]

(In reply to comment #16)
> Hot damn!  It will be very interesting to see what the leaks bot shows now that this huge leak is gone.

Unfortunately, it’s not much better :( We still have a lot of leaks.