Bug 43506 - foreign-iframe-main.html occasionally crashes (during the next test, idempotent-update.html)
Summary: foreign-iframe-main.html occasionally crashes (during the next test, idempote...
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac All
: P1 Normal
Assignee: Alexey Proskuryakov
Keywords: InRadar
: 45876 (view as bug list)
Depends on:
Blocks: 37812
  Show dependency treegraph
Reported: 2010-08-04 13:36 PDT by Joseph Pecoraro
Modified: 2010-10-04 15:44 PDT (History)
11 users (show)

See Also:

proposed fix (5.11 KB, patch)
2010-10-04 15:03 PDT, Alexey Proskuryakov
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2010-08-04 13:36:58 PDT
Leopard Bot has intermittently crashed twice on this test:

Most likely as a result of recent application cache changes in:
<http://webkit.org/b/40627> Limit ApplicationCache Total and Per-Origin Storage Capacity (Quotas)
Comment 1 Alexey Proskuryakov 2010-08-04 14:27:15 PDT
I think it's actually foreign-iframe-main.html that crashes the next test. I could get it to assert with --repeat-each 10:

Exception Codes: KERN_INVALID_ADDRESS at 0x00000000bbadbeef
Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   com.apple.WebKit              	0x00000001008be62d WebFrameLoaderClient::userAgent(WebCore::KURL const&) + 91 (WebFrameLoaderClient.mm:1281)
1   com.apple.WebCore             	0x00000001011ce685 WebCore::FrameLoader::applyUserAgent(WebCore::ResourceRequest&) + 65 (FrameLoader.cpp:3117)
2   com.apple.WebCore             	0x0000000100e55383 WebCore::ApplicationCacheGroup::createResourceHandle(WebCore::KURL const&, WebCore::ApplicationCacheResource*) + 109 (ApplicationCacheGroup.cpp:469)
3   com.apple.WebCore             	0x0000000100e56975 WebCore::ApplicationCacheGroup::startLoadingEntry() + 487 (ApplicationCacheGroup.cpp:997)
4   com.apple.WebCore             	0x0000000100e58e0c WebCore::ApplicationCacheGroup::didFinishLoading(WebCore::ResourceHandle*) + 672 (ApplicationCacheGroup.cpp:644)
5   com.apple.WebCore             	0x00000001018791ad -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] + 270 (ResourceHandleMac.mm:915)

This seems to mean that network loading didn't stop when a navigation happened, which may have far reaching consequences. I'm not sure if r40627 has anything to do with this.

This particular assertion doesn't mean immediately crashing in release mode, as there is an early return, but further processing of a spurious didFinishLoading can cause much badness.
Comment 2 Joseph Pecoraro 2010-08-04 14:49:49 PDT
(In reply to comment #1)
> I think it's actually foreign-iframe-main.html that crashes the next test.
> I could get it to assert with --repeat-each 10:

Thanks for looking into this Alexey! I just came back from a meeting
to see the same on my machine running a similar set of switches.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x00000000bbadbeef
0x00000001008be2d9 in WebFrameLoaderClient::userAgent (this=0x105a24dd0, url=@0x7fff5fbfde18) at /Users/pecoraro/Code/webkit-open-source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:1281
1281	    ASSERT(webView);
Comment 3 Alexey Proskuryakov 2010-08-09 02:38:30 PDT
On this test, appcache updating doesn't stop in time - the process continues after the frame is detached from view.

Normally, loading is stopped from ApplicationCacheGroup destructor, in a way that's anything but straightforward - even Inspector gets involved:

#0	0x10344801f in WebCore::ResourceHandle::cancel at ResourceHandleMac.mm:342
#1	0x102a19dbc in WebCore::ApplicationCacheGroup::stopLoading at ApplicationCacheGroup.cpp:336
#2	0x102a1d707 in WebCore::ApplicationCacheGroup::~ApplicationCacheGroup at ApplicationCacheGroup.cpp:95
#3	0x102a18be3 in WebCore::ApplicationCacheGroup::disassociateDocumentLoader at ApplicationCacheGroup.cpp:361
#4	0x102a26de7 in WebCore::ApplicationCacheHost::~ApplicationCacheHost at ApplicationCacheHost.cpp:59
#5	0x102c43a85 in WTF::deleteOwnedPtr<WebCore::ApplicationCacheHost> at OwnPtrCommon.h:57
#6	0x102c43aa8 in WTF::OwnPtr<WebCore::ApplicationCacheHost>::~OwnPtr at OwnPtr.h:57
#7	0x102c3f3dd in WebCore::DocumentLoader::~DocumentLoader at DocumentLoader.cpp:104
#8	0x10246c5c0 in WebDocumentLoaderMac::~WebDocumentLoaderMac at WebDocumentLoaderMac.h:40
#9	0x102a1ff39 in WTF::RefCounted<WebCore::DocumentLoader>::deref at RefCounted.h:139
#10	0x102a2303f in WTF::derefIfNotNull<WebCore::DocumentLoader> at PassRefPtr.h:58
#11	0x102a2305a in WTF::RefPtr<WebCore::DocumentLoader>::~RefPtr at RefPtr.h:56
#12	0x102f691da in WebCore::InspectorResource::~InspectorResource at InspectorResource.cpp:74
#13	0x102f21b4d in WTF::RefCounted<WebCore::InspectorResource>::deref at RefCounted.h:139
#14	0x102f21b7d in WTF::derefIfNotNull<WebCore::InspectorResource> at PassRefPtr.h:58
#15	0x102f21b98 in WTF::RefPtr<WebCore::InspectorResource>::~RefPtr at RefPtr.h:56
#16	0x102f185f7 in WebCore::InspectorController::~InspectorController at InspectorController.cpp:194
#17	0x1032b8b58 in WTF::deleteOwnedPtr<WebCore::InspectorController> at OwnPtrCommon.h:57
#18	0x1032b8b7c in WTF::OwnPtr<WebCore::InspectorController>::~OwnPtr at OwnPtr.h:57
#19	0x1032b35d4 in WebCore::Page::~Page at Page.cpp:224

It's possible that some subtle change in Inspector changed when ApplicationCacheGroup is destroyed (or maybe it's now leaked altogether). We need a more direct mechanism to stop updating.

This is also related to a FIXME in ApplicationCacheGroup.h:

    // FIXME: An update started by a particular frame should not stop if it is destroyed, but there are other frames associated with the same cache group.
    Frame* m_frame;
Comment 4 David Kilzer (:ddkilzer) 2010-08-09 14:50:31 PDT
Comment 5 Alexey Proskuryakov 2010-09-16 08:15:04 PDT
*** Bug 45876 has been marked as a duplicate of this bug. ***
Comment 6 Alexey Proskuryakov 2010-10-04 15:03:00 PDT
Created attachment 69694 [details]
proposed fix
Comment 7 Adam Barth 2010-10-04 15:05:38 PDT
Comment on attachment 69694 [details]
proposed fix

Comment 8 Alexey Proskuryakov 2010-10-04 15:20:19 PDT
Committed <http://trac.webkit.org/changeset/69041>.
Comment 9 WebKit Review Bot 2010-10-04 15:25:59 PDT
http://trac.webkit.org/changeset/69041 might have broken Chromium Linux Release
Comment 10 Alexey Proskuryakov 2010-10-04 15:42:19 PDT
This is forked code that doesn't build, I think it's up to chromium maintainers to fix it.
Comment 11 Adam Barth 2010-10-04 15:44:13 PDT
(In reply to comment #10)
> This is forked code that doesn't build, I think it's up to chromium maintainers to fix it.

Adding Chromium gardener folks to the CC.