Created attachment 243311 [details]
Instruments trace leading up to the crash.
I'm using WebKit in an iOS app (via UIWebView), and we're seeing a semi-frequent crash that I'm trying to track down. From the backtraces, I _think_ it appears to be a WebKit bug, and so I'd like to try to find a workaround, and/or submit a useful bug or patch.
A full thread dump is available here: http://crashes.to/s/cf0cdb52701
The assertion appears to be happening when the WebThread tries to call my delegate to decide whether to load a URL:
Thread : Crashed: WebThread
3 WebCore 0x31cd3061 SendDelegateMessage(NSInvocation*) + 184
4 WebKitLegacy 0x327be1f5 -[_WebSafeForwarder forwardInvocation:] + 116
5 CoreFoundation 0x269d766f ___forwarding___ + 354
6 CoreFoundation 0x26909058 _CF_forwarding_prep_0 + 24
7 WebKitLegacy 0x327ffb01 WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(WebCore::NavigationAction const&, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::PolicyAction)>) + 344
This crash becomes much easier to trigger when setting JSC_slowPathAllocsBetweenGCs to a low number.
I've attached an Instruments trace leading up to the crash, which I believe verifies that the app is not accessing the UIWebView on any non-main thread.
I've got a bit of new information to share. We released a new beta of the app in which this crash went from infrequent to very frequent (from 8 crashes in ~6k sessions to 160 crashes in ~5k sessions).
This was interesting because while the stack trace of the crashed thread varied, 156 of the 160 crashes showed the same backtrace for the main thread:
Thread : com.apple.main-thread
0 libsystem_kernel.dylib 0x2fe05ba8 __psynch_mutexwait + 24
1 libsystem_pthread.dylib 0x2fe8104b _pthread_mutex_lock + 398
2 WebCore 0x2d353b91 _WebTryThreadLock(bool) + 44
3 WebCore 0x2d3543ad WebThreadLock + 80
4 UIKit 0x2561ca69 -[UIWebDocumentView setDataDetectorTypes:] + 56
5 Boxer 0x000cc245 -[ConversationViewController renderConversation] (ConversationViewController.m:869)
6 libdispatch.dylib 0x2fd1f7bb _dispatch_call_block_and_release + 10
7 libdispatch.dylib 0x2fd1f7a7 _dispatch_client_callout + 22
8 libdispatch.dylib 0x2fd22fa3 _dispatch_main_queue_callback_4CF + 718
9 CoreFoundation 0x21fcf3b1 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 8
10 CoreFoundation 0x21fcdab1 __CFRunLoopRun + 1512
11 CoreFoundation 0x21f1b3c1 CFRunLoopRunSpecific + 476
12 CoreFoundation 0x21f1b1d3 CFRunLoopRunInMode + 106
13 GraphicsServices 0x293190a9 GSEventRunModal + 136
14 UIKit 0x2552afa1 UIApplicationMain + 1440
15 Boxer 0x000900a7 main (main.m:11)
(More details here: http://crashes.to/s/b1531e29971)
I looked through the Instruments trace and it convinced me of something that, in retrospect, is obvious just from the crashing backtrace: It's possible for SendDelegateMessage to DropAllLocks when it doesn't hold the lock and, if it does so while the main thread is garbage collecting, this RELEASE_ASSERT will file.
I think the right solution here is to refactor this RELEASE_ASSERT not to fire under this condition, since the condition is otherwise harmless.
Thanks, Geoff! I think that workaround will work for us, really appreciate you taking a look at this. If there is anything else I can do to help lock this one down let me know.
Created attachment 254382 [details]
Comment on attachment 254382 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=254382&action=review
> + RELEASE_ASSERT(!currentThreadIsHoldingLock() || !m_vm->isCollectorBusy());
Shouldn’t this be
RELEASE_ASSERT(currentThreadIsHoldingLock() && !m_vm->isCollectorBusy());
The current thread shouldn’t be allowed to drop locks if it doesn’t own it, or if GC is in progress.
> The current thread shouldn’t be allowed to drop locks if it doesn’t own it
Yes it should be:
(JSC::JSLock::dropAllLocks): Removed a comment because it duplicated
the code beneath it. Removed a FIXME because we can't ASSERT that
we're holding the lock. WebKit1 on iOS drops the lock before calling to
delegates, not knowing whether it holds the lock or not.
Created attachment 254386 [details]
New patch to fix the build.
Note that dropping the lock when you do not hold it is a no-op. (You are not allowed to drop someone else's lock.)
(In reply to comment #11)
> Note that dropping the lock when you do not hold it is a no-op. (You are not
> allowed to drop someone else's lock.)
That’s what I was missing!
Comment on attachment 254386 [details]
Committed r185268: <http://trac.webkit.org/changeset/185268>