Bug 166986 - ASSERTION FAILED: !m_bodyLoader
Summary: ASSERTION FAILED: !m_bodyLoader
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-12 14:02 PST by Keith Rollin
Modified: 2017-02-13 16:17 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.87 KB, patch)
2017-01-19 17:13 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Rollin 2017-01-12 14:02:14 PST
This assertion occasionally triggers when running a new PLT mechanism that's under development. As part of this mechanism, a number of pages are being loaded in sequence, including:

...
http://www.Facebook.com
http://www.Fc2.com
http://www.Github.com
http://www.Globo.com
http://www.Godaddy.com
http://www.Google.com
http://www.Huffingtonpost.com
http://www.Imdb.com
...

I find that the ASSERT usually triggers when loading huffingtonpost.com, but I have at least one instance of globo.com crashing. Since the ASSERT is related to cancelling a loader, it's possible that the issue is with the previous page (github.com or google.com).

The backtrace of the ASSERT is:

ASSERTION FAILED: !m_bodyLoader
/Volumes/Data/dev/WebKit/branches/record_playback/OpenSource/Source/WebCore/Modules/fetch/FetchResponse.cpp(340) : virtual void WebCore::FetchResponse::stop()
1   0x114b73d5d WTFCrash
2   0x117d41e8c WebCore::FetchResponse::stop()
3   0x11980e58e WebCore::ScriptExecutionContext::stopActiveDOMObjects()
4   0x117b0f2a5 WebCore::Document::stopActiveDOMObjects()
5   0x117b0efb5 WebCore::Document::prepareForDestruction()
6   0x1176512b9 WebCore::CachedFrame::destroy()
7   0x117662379 WebCore::CachedPage::~CachedPage()
8   0x117662465 WebCore::CachedPage::~CachedPage()
9   0x11928f63c WebCore::PageCache::prune(WebCore::PruningReason)
10  0x1192902bf WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*)
11  0x117ebf487 WebCore::FrameLoader::commitProvisionalLoad()
12  0x117b8c47c WebCore::DocumentLoader::commitIfReady()
13  0x117b8fe9c WebCore::DocumentLoader::commitLoad(char const*, int)
14  0x117b8fe47 WebCore::DocumentLoader::dataReceived(char const*, int)
15  0x117b90554 WebCore::DocumentLoader::dataReceived(WebCore::CachedResource&, char const*, int)
16  0x117662d88 WebCore::CachedRawResource::notifyClientsDataWasReceived(char const*, unsigned int)
17  0x117662c12 WebCore::CachedRawResource::addDataBuffer(WebCore::SharedBuffer&)
18  0x119b09c87 WebCore::SubresourceLoader::didReceiveDataOrBuffer(char const*, int, WTF::RefPtr<WebCore::SharedBuffer>&&, long long, WebCore::DataPayloadType)
19  0x119b09a92 WebCore::SubresourceLoader::didReceiveData(char const*, unsigned int, long long, WebCore::DataPayloadType)
20  0x10ddbb998 WebKit::WebResourceLoader::didReceiveData(IPC::DataReference const&, long long)
21  0x10ddc029c void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, 0ul, 1ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>)
22  0x10ddc0078 void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long), std::__1::tuple<IPC::DataReference, long long>, std::__1::integer_sequence<unsigned long, 0ul, 1ul> >(std::__1::tuple<IPC::DataReference, long long>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long))
23  0x10ddbf69d void IPC::handleMessage<Messages::WebResourceLoader::DidReceiveData, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long)>(IPC::Decoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(IPC::DataReference const&, long long))
24  0x10ddbeff6 WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&)
25  0x10d65bd5d WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
26  0x10d3e6783 IPC::Connection::dispatchMessage(IPC::Decoder&)
27  0x10d3dbd28 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)
28  0x10d3e6d80 IPC::Connection::dispatchOneMessage()
29  0x10d3ffa0d IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()()
30  0x10d3ff969 WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call()
31  0x114ba06de WTF::Function<void ()>::operator()() const

The ASSERT was added in commit 51223ca642f79c170c43408c3c57b22ba73ec220, svn r199641, which made this change:

void FetchResponse::stop()
{
+    RefPtr<FetchResponse> protect(this);
    FetchBodyOwner::stop();
    if (m_bodyLoader) {
-        RefPtr<FetchResponse> protect(this);
        m_bodyLoader->stop();
-        m_bodyLoader = Nullopt;
+        ASSERT(!m_bodyLoader);
    }
}
Comment 1 youenn fablet 2017-01-12 14:20:23 PST
Thanks for the information.
This ASSERT is important to ensure that m_bodyLoader is nullified and we unset pending activity correctly.

I think some refactoring of the code might help though.
For instance, it might be clearer to  set/unset pending activities in body loader constructor/destructor.
Comment 2 youenn fablet 2017-01-19 17:13:53 PST
Created attachment 299289 [details]
Patch
Comment 3 youenn fablet 2017-01-19 17:29:55 PST
Keith, would you be able to test this patch in your environment?
Comment 4 Keith Rollin 2017-01-19 17:48:13 PST
(In reply to comment #3)
> Keith, would you be able to test this patch in your environment?

Sure.
Comment 5 Keith Rollin 2017-01-19 18:51:28 PST
(In reply to comment #4)
> (In reply to comment #3)
> > Keith, would you be able to test this patch in your environment?
> 
> Sure.

Looks good. No crashes.
Comment 6 WebKit Commit Bot 2017-02-13 16:17:48 PST
Comment on attachment 299289 [details]
Patch

Clearing flags on attachment: 299289

Committed r212257: <http://trac.webkit.org/changeset/212257>
Comment 7 WebKit Commit Bot 2017-02-13 16:17:53 PST
All reviewed patches have been landed.  Closing bug.