Bug 205888 – [iOS] Crash when running WebKit legacy layout tests on device

• Status: NEW
• Component: WebKit Misc.

[Per Arne Vollan]

Thread 0 Crashed:
0   JavaScriptCore                	0x0000000117312128 WTFCrash + 28
1   WebKitLegacy                  	0x000000012bfad6ec WTF::RefCountedBase::applyRefDerefThreadingCheck() const + 168
2   WebKitLegacy                  	0x000000012bfad4c0 WTF::RefCountedBase::derefBase() const + 32
3   WebKitLegacy                  	0x000000012c158684 WTF::RefCounted<LayerFlushController, std::__1::default_delete<LayerFlushController> >::deref() const + 32
4   WebKitLegacy                  	0x000000012c15c270 void WTF::derefIfNotNull<LayerFlushController>(LayerFlushController*) + 52
5   WebKitLegacy                  	0x000000012c16256c WTF::RefPtr<LayerFlushController, WTF::DumbPtrTraits<LayerFlushController> >::~RefPtr() + 48
6   WebKitLegacy                  	0x000000012c1623a8 WTF::RefPtr<LayerFlushController, WTF::DumbPtrTraits<LayerFlushController> >::~RefPtr() + 32
7   WebKitLegacy                  	0x000000012c1f8078 WebViewLayerFlushScheduler::layerFlushCallback() + 136
8   WebKitLegacy                  	0x000000012c1f8ef8 WebViewLayerFlushScheduler::WebViewLayerFlushScheduler(LayerFlushController*)::$_0::operator()() const + 28
9   WebKitLegacy                  	0x000000012c1f8e98 WTF::Detail::CallableWrapper<WebViewLayerFlushScheduler::WebViewLayerFlushScheduler(LayerFlushController*)::$_0, void>::call() + 28
10  WebCore                       	0x000000010394d2c0 WTF::Function<void ()>::operator()() const + 128
11  WebCore                       	0x0000000104de9eac WebCore::RunLoopObserver::runLoopObserverFired() + 124
12  WebCore                       	0x0000000104de9e24 WebCore::RunLoopObserver::runLoopObserverFired(__CFRunLoopObserver*, unsigned long, void*) + 36
13  CoreFoundation                	0x00000001aad43210 __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 32
14  CoreFoundation                	0x00000001aad3e104 __CFRunLoopDoObservers + 420
15  CoreFoundation                	0x00000001aad3e580 __CFRunLoopRun + 968
16  CoreFoundation                	0x00000001aad3de8c CFRunLoopRunSpecific + 424
17  DumpRenderTree                	0x000000010083ff74 runTest(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 2600
18  DumpRenderTree                	0x000000010083f4b0 runTestingServerLoop() + 168
19  DumpRenderTree                	0x000000010083ee54 dumpRenderTree(int, char const**) + 772
20  DumpRenderTree                	0x000000010084078c -[DumpRenderTree _runDumpRenderTree] + 48
21  Foundation                    	0x00000001ab1ac398 __NSThreadPerformPerform + 184
22  CoreFoundation                	0x00000001aad43dbc __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 24
23  CoreFoundation                	0x00000001aad43d14 __CFRunLoopDoSource0 + 80
24  CoreFoundation                	0x00000001aad434f8 __CFRunLoopDoSources0 + 276
25  CoreFoundation                	0x00000001aad3e4cc __CFRunLoopRun + 788
26  CoreFoundation                	0x00000001aad3de8c CFRunLoopRunSpecific + 424
27  GraphicsServices              	0x00000001b53a738c GSEventRunModal + 160
28  UIKitCore                     	0x00000001aee4fbb4 UIApplicationMain + 1932
29  DumpRenderTree                	0x0000000100840cc8 DumpRenderTreeMain(int, char const**) + 148
30  DumpRenderTree                	0x00000001008aecdc main + 36
31  libdyld.dylib                 	0x00000001aabc5810 start + 4

[Per Arne Vollan]

Created attachment 387036 [details]
Patch

[Brent Fulgham]

Comment on attachment 387036 [details]
Patch

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

r=me

> Source/WebKitLegacy/mac/WebView/WebViewData.h:127
> +class LayerFlushController : public ThreadSafeRefCounted<LayerFlushController> {

Whoops!

[Brent Fulgham]

Comment on attachment 387036 [details]
Patch

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

> Source/WebKitLegacy/mac/ChangeLog:9
> +        and another thread.

Is that expected? Should all flushing happen on main thread?

[Brent Fulgham]

Comment on attachment 387036 [details]
Patch

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

>> Source/WebKitLegacy/mac/ChangeLog:9
>> +        and another thread.
> 
> Is that expected? Should all flushing happen on main thread?

You might check with Tim or Simon to make sure this is expected.

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #4)
> Comment on attachment 387036 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387036&action=review
> 
> >> Source/WebKitLegacy/mac/ChangeLog:9
> >> +        and another thread.
> > 
> > Is that expected? Should all flushing happen on main thread?
> 
> You might check with Tim or Simon to make sure this is expected.

Tim, Simon, Said, is this the correct way to fix this issue?

[Simon Fraser (smfr)]

What's the "other" thread? Is it the WebThread?

[Simon Fraser (smfr)]

There are plenty of ref-counted things that are touched from the main thread and the web thread, but WebThreadLock should protect them. Why is LayerFlushScheduler different?

[Per Arne Vollan]

(In reply to Simon Fraser (smfr) from comment #7)
> There are plenty of ref-counted things that are touched from the main thread
> and the web thread, but WebThreadLock should protect them. Why is
> LayerFlushScheduler different?

Ah, so this might not be a real problem if the WebThreadLock is protecting LayerFlushScheduler? At the moment, I am not sure what other thread is accessing the LayerFlushScheduler. It would still be good to fix the assert, though.

[Tim Horton]

isMainThread includes the Web Thread and the main thread, so this means some third thread is doing reffing or dereffing LayerFlushScheduler, which suggests you're papering over the problem.

[Tim Horton]

Actually, suggests it was *created* on a third thread, since the crash is in deref

[Per Arne Vollan]

(In reply to Tim Horton from comment #9)
> isMainThread includes the Web Thread and the main thread, so this means some
> third thread is doing reffing or dereffing LayerFlushScheduler, which
> suggests you're papering over the problem.

That makes sense, I will try to find the third thread.

Thanks for reviewing, all!

[Brent Fulgham]

Comment on attachment 387036 [details]
Patch

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

> Source/WebKitLegacy/mac/WebView/WebViewData.h:129
>      static Ref<LayerFlushController> create(WebView* webView)

Could you add an RELEASE_ASSERT(isMainThread()) here to find the bad guy?

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #12)
> Comment on attachment 387036 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=387036&action=review
> 
> > Source/WebKitLegacy/mac/WebView/WebViewData.h:129
> >      static Ref<LayerFlushController> create(WebView* webView)
> 
> Could you add an RELEASE_ASSERT(isMainThread()) here to find the bad guy?

Good point! I will try that.