Bug 148890

Summary: http/tests/contentfiltering/load-substitute-data-from-appcache.html and http/tests/appcache/decide-navigation-policy-after-delay.html crash in DocumentLoader::dataReceived sometimes
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: WebCore Misc.Assignee: Andy Estes <aestes>
Status: NEW ---    
Severity: Normal CC: aestes, buildbot, ddkilzer, rniwa, ryanhaddad
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
Archive of layout-test-results from ews102 for mac-mavericks
add a similar test to isolate the problem ap: review+

Description Alexey Proskuryakov 2015-09-04 23:40:47 PDT
I saw http/tests/contentfiltering/load-substitute-data-from-appcache.html (a new test) crash several times on Yosemite with a release assert:


0   com.apple.JavaScriptCore      	0x000000010cbb4797 WTFCrash + 39
1   com.apple.WebCore             	0x000000010eb4e4ce WebCore::DocumentLoader::dataReceived(WebCore::CachedResource*, char const*, int) + 206 (DocumentLoader.cpp:864)
2   com.apple.WebCore             	0x000000010eb4dfed WebCore::DocumentLoader::continueAfterContentPolicy(WebCore::PolicyAction) + 1773 (DocumentLoader.cpp:753)
3   com.apple.WebCore             	0x000000010eb4d680 WebCore::DocumentLoader::responseReceived(WebCore::CachedResource*, WebCore::ResourceResponse const&) + 1968 (DocumentLoader.cpp:660)
4   com.apple.WebCore             	0x000000010e841f20 WebCore::ContentFilter::responseReceived(WebCore::CachedResource*, WebCore::ResourceResponse const&) + 432 (ContentFilter.cpp:163)
5   com.apple.WebCore             	0x000000010e70e189 WebCore::CachedRawResource::responseReceived(WebCore::ResourceResponse const&) + 201 (CachedRawResource.cpp:185)
6   com.apple.WebCore             	0x000000011040475f WebCore::SubresourceLoader::didReceiveResponse(WebCore::ResourceResponse const&) + 767 (SubresourceLoader.cpp:232)
7   com.apple.WebKit              	0x00000001098c4c92 WebKit::WebResourceLoader::didReceiveResponse(WebCore::ResourceResponse const&, bool) + 306 (WebResourceLoader.cpp:130)
Comment 1 Alexey Proskuryakov 2015-09-08 09:23:19 PDT
Comment 2 Alexey Proskuryakov 2015-09-16 16:01:53 PDT
Comment 3 Andy Estes 2015-09-20 13:12:41 PDT
I suspect this assertion would be hit even if Content Filtering wasn't enabled. The test decides navigation policy asynchronously in order mimic iOS Safari's behavior, so the response is sometimes received before navigation policy is decided. When loading substitute data, dataReceived() is called in continueAfterContentPolicy() with a null CachedResource, and it asserts that m_mainResource has already been set to 0 in continueAfterNavigationPolicy():

    ASSERT_UNUSED(resource, resource == m_mainResource);

I'm going to upload this same test without Content Filtering enabled to verify my assumption that the crash will still sometimes occur.
Comment 4 Andy Estes 2015-09-20 13:46:07 PDT
Created attachment 261611 [details]
Comment 5 Build Bot 2015-09-20 14:19:39 PDT
Comment on attachment 261611 [details]

Attachment 261611 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/190910

New failing tests:
Comment 6 Build Bot 2015-09-20 14:19:42 PDT
Created attachment 261614 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 7 Andy Estes 2015-09-20 14:32:19 PDT
Created attachment 261615 [details]
add a similar test to isolate the problem
Comment 8 Alexey Proskuryakov 2015-09-20 15:04:13 PDT
Comment on attachment 261615 [details]
add a similar test to isolate the problem

Is this completely unreproducible locally? What did you try?
Comment 9 Andy Estes 2015-09-20 15:07:53 PDT
(In reply to comment #8)
> Comment on attachment 261615 [details]
> Patch
> Is this completely unreproducible locally? What did you try?

I wasn't able to reproduce locally after running the tests in http/tests/contentfiltering/ for 100 iterations, but it seems to crash on the bots at  least once a day. I was planning to land this and check for crashes tomorrow.
Comment 10 Andy Estes 2015-09-20 15:10:15 PDT
I also tried with --order=random.
Comment 11 Alexey Proskuryakov 2015-09-20 16:22:57 PDT
It often helps to simulate full CPU load with --fully-parallel, possibly even overcommitting with --child-processes=<more than it normally starts>.

> I was planning to land this and check for crashes tomorrow.

Comment 12 Andy Estes 2015-09-20 21:26:59 PDT
Committed r190041: <http://trac.webkit.org/changeset/190041>
Comment 13 Alexey Proskuryakov 2015-10-04 13:39:29 PDT
*** Bug 149723 has been marked as a duplicate of this bug. ***
Comment 14 Alexey Proskuryakov 2015-10-04 13:48:24 PDT
Marked both tests as flaky in r190552. It was tricky since they had complicated and different skip/unskip chains.
Comment 15 Alexey Proskuryakov 2016-12-15 14:41:11 PST
This seems like it stopped happening some time earlier in 2016.
Comment 16 Alexey Proskuryakov 2016-12-15 14:42:41 PST
(and then these same tests started to crash in a different way, frequently)