Bug 140973

Summary: Web Inspector: Crash closing inspected page with frequent activity
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: DoNotImportToRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
[ARCHIVE] Test Archive
none
[PATCH] Proposed Fix
none
[PATCH] Proposed Fix none

Description Joseph Pecoraro 2015-01-27 18:28:53 PST
Created attachment 245500 [details]
[ARCHIVE] Test Archive

* SUMMARY
WebContent crash closing inspected page with frequent activity.

* STEPS
1. Open the_mind_of_a_con_man.webarchive
2. Show Reader
3. Inspect Reader page (⌥⌘C)
4. Hide Reader
  => CRASH (repeat 3-4 if not crash)

* NOTES
- Crashes can be anywhere under WebPage::close(), when any Dispatcher attempts to use the InspectorFrontendChannel (WebInspector object that was just destroyed in WebPage::close without disconnecting).

* CRASH 1

    Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
    Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000013647

    Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
    0   com.apple.JavaScriptCore      	0x00007fff83be0235 Inspector::InspectorNetworkFrontendDispatcher::loadingFailed(WTF::String const&, double, WTF::String const&, bool const*) + 1077
    1   com.apple.WebCore             	0x00007fff844bdedf WebCore::InspectorResourceAgent::didFailLoading(unsigned long, WebCore::DocumentLoader*, WebCore::ResourceError const&) + 431
    2   com.apple.WebCore             	0x00007fff83f8ff7c WebCore::InspectorInstrumentation::didFailLoadingImpl(WebCore::InstrumentingAgents*, unsigned long, WebCore::DocumentLoader*, WebCore::ResourceError const&) + 76
    3   com.apple.WebCore             	0x00007fff83fd9cca WebCore::ResourceLoader::cancel(WebCore::ResourceError const&) + 426
    4   com.apple.WebCore             	0x00007fff83fd9b06 WebCore::ResourceLoader::cancel() + 70
    5   com.apple.WebCore             	0x00007fff84279098 WebCore::cancelAll(WTF::HashMap<unsigned long, WTF::RefPtr<WebCore::ResourceLoader>, WTF::IntHash<unsigned long>, WTF::HashTraits<unsigned long>, WTF::HashTraits<WTF::RefPtr<WebCore::ResourceLoader> > > const&) + 296
    6   com.apple.WebCore             	0x00007fff83e48ad1 WebCore::DocumentLoader::stopLoading() + 1361
    7   com.apple.WebCore             	0x00007fff83e484d0 WebCore::FrameLoader::stopAllLoaders(WebCore::ClearProvisionalItemPolicy) + 208
    8   com.apple.WebCore             	0x00007fff83efa85a WebCore::FrameLoader::detachFromParent() + 122
    9   com.apple.WebCore             	0x00007fff83ddabf5 WebCore::FrameLoader::detachChildren() + 245
    10  com.apple.WebCore             	0x00007fff83efa850 WebCore::FrameLoader::detachFromParent() + 112
    11  com.apple.WebKit              	0x00007fff9250d7f7 WebKit::WebPage::close() + 751
    ...


* CRASH 2

    Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
    Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000010

    Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
    0   com.apple.JavaScriptCore      	0x00000001118b2eba Inspector::InspectorPageFrontendDispatcher::frameDetached(WTF::String const&) + 346
    1   com.apple.WebCore             	0x0000000114026856 WebCore::InspectorPageAgent::frameDetached(WebCore::Frame&) + 230 (InspectorPageAgent.cpp:735)
    2   com.apple.WebCore             	0x0000000113ffa928 WebCore::InspectorInstrumentation::frameDetachedFromParentImpl(WebCore::InstrumentingAgents&, WebCore::Frame&) + 56 (InspectorInstrumentation.cpp:747)
    3   com.apple.WebCore             	0x0000000113b9bfc4 WebCore::InspectorInstrumentation::frameDetachedFromParent(WebCore::Frame&) + 52 (InspectorInstrumentation.h:1029)
    4   com.apple.WebCore             	0x0000000113b8dead WebCore::FrameLoader::detachFromParent() + 157 (FrameLoader.cpp:2492)
    5   com.apple.WebCore             	0x0000000113b876f3 WebCore::FrameLoader::detachChildren() + 275 (FrameLoader.cpp:2409)
    6   com.apple.WebCore             	0x0000000113b8de8d WebCore::FrameLoader::detachFromParent() + 125 (FrameLoader.cpp:2484)
    7   com.apple.WebCore             	0x0000000113b876f3 WebCore::FrameLoader::detachChildren() + 275 (FrameLoader.cpp:2409)
    8   com.apple.WebCore             	0x0000000113b8de8d WebCore::FrameLoader::detachFromParent() + 125 (FrameLoader.cpp:2484)
    9   com.apple.WebCore             	0x0000000113b876f3 WebCore::FrameLoader::detachChildren() + 275 (FrameLoader.cpp:2409)
    10  com.apple.WebCore             	0x0000000113b8de8d WebCore::FrameLoader::detachFromParent() + 125 (FrameLoader.cpp:2484)
    11  com.apple.WebCore             	0x0000000113b876f3 WebCore::FrameLoader::detachChildren() + 275 (FrameLoader.cpp:2409)
    12  com.apple.WebCore             	0x0000000113b8de8d WebCore::FrameLoader::detachFromParent() + 125 (FrameLoader.cpp:2484)
    13  com.apple.WebKit              	0x000000010ebc6f54 WebKit::WebPage::close() + 3364 (WebPage.cpp:1031)
    ...
Comment 1 Joseph Pecoraro 2015-01-27 18:29:10 PST
<rdar://problem/15833013>
Comment 2 Joseph Pecoraro 2015-01-27 18:48:18 PST
Created attachment 245502 [details]
[PATCH] Proposed Fix
Comment 3 Timothy Hatcher 2015-01-27 20:50:10 PST
Comment on attachment 245502 [details]
[PATCH] Proposed Fix

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

> Source/WebKit2/WebProcess/WebPage/WebInspector.h:90
> +    virtual ~WebInspector() { close(); }

Usually we add a method like webPageDestroyed() and call it from ~WebPage. Just because the WebPage is destroyed doesn't mean it was he last reference for WebInspector.
Comment 4 Timothy Hatcher 2015-01-27 20:52:29 PST
Comment on attachment 245502 [details]
[PATCH] Proposed Fix

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

>> Source/WebKit2/WebProcess/WebPage/WebInspector.h:90
>> +    virtual ~WebInspector() { close(); }
> 
> Usually we add a method like webPageDestroyed() and call it from ~WebPage. Just because the WebPage is destroyed doesn't mean it was he last reference for WebInspector.

Which you would call in WebPage::close, so it should cakes somthing like webPageClosed.
Comment 5 Joseph Pecoraro 2015-01-28 12:08:18 PST
Created attachment 245561 [details]
[PATCH] Proposed Fix
Comment 6 WebKit Commit Bot 2015-01-28 17:49:21 PST
Comment on attachment 245561 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 245561

Committed r179321: <http://trac.webkit.org/changeset/179321>
Comment 7 WebKit Commit Bot 2015-01-28 17:49:26 PST
All reviewed patches have been landed.  Closing bug.