Bug 145488

Summary:

Web Inspector: Crash closing a related tab with Web Inspector open while page is refreshing

Product:

WebKit

Reporter:

Joseph Pecoraro <joepeck>

Component:

Web Inspector

Assignee:

Joseph Pecoraro <joepeck>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, ap, graouts, joepeck, jonowells, mattbaker, nvasilyev, timothy, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
[PATCH] Proposed Fix	none
[PATCH] Proposed Fix	ap: review+

Joseph Pecoraro

Reported 2015-05-29 20:17:53 PDT

* SUMMARY Crash closing a related tab with Web Inspector open while page is refreshing. * STEPS TO REPRODUCE 1. Inspect <https://www.tumblr.com> 2. Expand <head> in DOM Tree 3. Click URL from <link rel="icon" sizes="any" mask="" href="https://secure.assets.tumblr.com/images/tumblr_t_logo.svg?_v=fe2b348bb13d366e9e93d5bf6a8b2fdd"> to open new page in the same WebProcess 4. Refresh the tumblr.com page with the Web Inspector 5. Close Tab (⌘W) while the page is refreshing => occasionally a CRASH * CRASH Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: EXC_I386_GPFLT Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.WebKit std::__1::__function::__func<IPC::Connection::connectionDidClose()::$_8, std::__1::allocator<IPC::Connection::connectionDidClose()::$_8>, void ()>::operator()() + 28 1 com.apple.JavaScriptCore WTF::RunLoop::performWork() + 850 (RunLoop.cpp:106) 2 com.apple.JavaScriptCore WTF::RunLoop::performWork(void*) + 34 (RunLoopCF.cpp:39) 3 com.apple.CoreFoundation __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17 4 com.apple.CoreFoundation __CFRunLoopDoSources0 + 269 5 com.apple.CoreFoundation __CFRunLoopRun + 927 6 com.apple.CoreFoundation CFRunLoopRunSpecific + 296 7 com.apple.HIToolbox RunCurrentEventLoopInMode + 235 8 com.apple.HIToolbox ReceiveNextEventCommon + 431 9 com.apple.HIToolbox _BlockUntilNextEventMatchingListInModeWithFilter + 71 10 com.apple.AppKit _DPSNextEvent + 978 11 com.apple.AppKit -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 346 12 com.apple.AppKit -[NSApplication run] + 594 13 com.apple.AppKit NSApplicationMain + 1832 14 libxpc.dylib _xpc_objc_main + 793 15 libxpc.dylib xpc_main + 490 16 com.apple.WebKit.WebContent.Development main + 21 (XPCServiceMain.Development.mm:90) 17 libdyld.dylib start + 1 * LLDB (Release) - Looks like the client is a garbage object, it never invalidated the connection properly. (lldb) Process 33413 stopped * thread #1: tid = 0x20b823, 0x0000000109ea6c8e WebKit`std::__1::__function::__func<IPC::Connection::connectionDidClose()::$_8, std::__1::allocator<IPC::Connection::connectionDidClose()::$_8>, void ()>::operator()() [inlined] IPC::Connection::connectionDidClose()::$_8::operator()() const + 24 at Connection.cpp:781, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT) frame #0: 0x0000000109ea6c8e WebKit`std::__1::__function::__func<IPC::Connection::connectionDidClose()::$_8, std::__1::allocator<IPC::Connection::connectionDidClose()::$_8>, void ()>::operator()() [inlined] IPC::Connection::connectionDidClose()::$_8::operator()() const + 24 at Connection.cpp:781 778 Client* client = connection->m_client; 779 connection->m_client = nullptr; 780 -> 781 client->didClose(*connection); 782 }); 783 } 784 (lldb) p client (IPC::Connection::Client *) $0 = 0x00007fcfd0529118 (lldb) p client (IPC::Connection::Client) $11 = {}

Attachments
[PATCH] Proposed Fix (1.98 KB, patch) 2015-05-29 20:24 PDT, Joseph Pecoraro	no flags	Details Formatted Diff Diff
[PATCH] Proposed Fix (1.98 KB, patch) 2015-05-29 20:25 PDT, Joseph Pecoraro	ap: review+	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Joseph Pecoraro

Comment 1 2015-05-29 20:18:23 PDT

<rdar://problem/21142346>

Joseph Pecoraro

Comment 2 2015-05-29 20:21:16 PDT

FWIW, the case where we would close without invalidating is under WebPage::close(): * thread #1: tid = 0x246321, 0x00000001093baa81 WebKit`WebKit::WebInspector::~WebInspector(this=0x00007ffce6442a58) + 71 at WebInspector.cpp:78, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 * frame #0: 0x00000001093baa81 WebKit`WebKit::WebInspector::~WebInspector(this=0x00007ffce6442a58) + 71 at WebInspector.cpp:78 frame #1: 0x000000010949e506 WebKit`-[WKObject dealloc](self=0x00007ffce6442a40, _cmd=<unavailable>) + 25 at WKObject.mm:40 frame #2: 0x00007fff99d2e89c libobjc.A.dylib`objc_object::sidetable_release(bool) + 236 frame #3: 0x00007fff8c942db0 CoreFoundation`CFRelease + 304 frame #4: 0x00000001093d9c7f WebKit`WebKit::WebPage::close() [inlined] void WTF::derefIfNotNull<WebKit::WebInspector>(ptr=<unavailable>) + 231 at PassRefPtr.h:42 frame #5: 0x00000001093d9c75 WebKit`WebKit::WebPage::close() [inlined] WTF::RefPtr<WebKit::WebInspector>::~RefPtr() at RefPtr.h:59 frame #6: 0x00000001093d9c75 WebKit`WebKit::WebPage::close() [inlined] WTF::RefPtr<WebKit::WebInspector>::~RefPtr() at RefPtr.h:59 frame #7: 0x00000001093d9c75 WebKit`WebKit::WebPage::close() [inlined] WTF::RefPtr<WebKit::WebInspector>::operator=(WebKit::WebInspector*) + 18 at RefPtr.h:142 frame #8: 0x00000001093d9c63 WebKit`WebKit::WebPage::close(this=0x00007ffce6809410) + 203 at WebPage.cpp:925 frame #9: 0x00000001093f4cad WebKit`WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::MessageDecoder&) [inlined] void IPC::callMemberFunctionImpl<WebKit::WebPage, void (WebKit::WebPage::*)(), std::__1::tuple<> >(object=0x00007ffce6809410)(), std::__1::tuple<>&&, std::index_sequence<>) + 8 at HandleMessage.h:16 frame #10: 0x00000001093f4ca5 WebKit`WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::MessageDecoder&) [inlined] void IPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(), std::__1::tuple<>, std::make_index_sequence<0ul> >(std::__1::tuple<>&&, WebKit::WebPage*, void (WebKit::WebPage::*)()) at HandleMessage.h:22 frame #11: 0x00000001093f4ca5 WebKit`WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::MessageDecoder&) [inlined] void IPC::handleMessage<Messages::WebPage::Close, WebKit::WebPage, void (WebKit::WebPage::*)()>(decoder=<unavailable>)()) at HandleMessage.h:92 frame #12: 0x00000001093f4ca5 WebKit`WebKit::WebPage::didReceiveWebPageMessage(this=0x00007ffce6809410, connection=<unavailable>, decoder=<unavailable>) + 5585 at WebPageMessageReceiver.cpp:655 This does: if (m_inspector) { m_inspector->disconnectFromPage(); m_inspector = nullptr; } And disconnectFromPage just does close() without going further to invalidate. We could make disconnectFromPage invalidate, but I figure putting it in the destructor we are guaranteed it will happen no matter how we destruct.

Joseph Pecoraro

Comment 3 2015-05-29 20:24:24 PDT

Created attachment 253948 [details] [PATCH] Proposed Fix

Joseph Pecoraro

Comment 4 2015-05-29 20:25:47 PDT

Created attachment 253949 [details] [PATCH] Proposed Fix

Joseph Pecoraro

Comment 5 2015-05-29 20:56:02 PDT

Committed r185030: <http://trac.webkit.org/changeset/185030>

Note You need to log in before you can comment on or make changes to this bug.