ScrollingStateNode is not ThreadSafeRefCounted but is ref'd / deref'd from several threads

[Chris Dumez]

ScrollingStateNode is not ThreadSafeRefCounted but is ref'd / deref'd from several threads:

Thread 8 Crashed:: WebCore: Scrolling
0   com.apple.WebCore             	0x0000000101a45093 WTFCrashWithInfo(int, char const*, char const*, int) + 19
1   com.apple.WebCore             	0x0000000102cfddce WTF::RefCounted<WebCore::ScrollingStateNode>::deref() const + 126
2   com.apple.WebCore             	0x0000000102d00188 WebCore::ScrollingStateTree::~ScrollingStateTree() + 40
3   com.apple.WebCore             	0x0000000102d07b80 WebCore::ThreadedScrollingTree::commitTreeState(std::__1::unique_ptr<WebCore::ScrollingStateTree, std::__1::default_delete<WebCore::ScrollingStateTree> >) + 64
4   com.apple.WebCore             	0x0000000101c467e7 WTF::Detail::CallableWrapper<WebCore::ScrollingCoordinatorMac::commitTreeState()::$_5, void>::call() + 39
5   com.apple.WebCore             	0x0000000102d01d79 WebCore::ScrollingThread::dispatchFunctionsFromScrollingThread() + 121
6   com.apple.WebCore             	0x0000000101c4584a WebCore::ScrollingThread::threadRunLoopSourceCallback(void*) + 26
7   com.apple.CoreFoundation      	0x00007fff3e2f8581 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
8   com.apple.CoreFoundation      	0x00007fff3e3b08ac __CFRunLoopDoSource0 + 108
9   com.apple.CoreFoundation      	0x00007fff3e2db530 __CFRunLoopDoSources0 + 208
10  com.apple.CoreFoundation      	0x00007fff3e2da9ad __CFRunLoopRun + 1293
11  com.apple.CoreFoundation      	0x00007fff3e2da207 CFRunLoopRunSpecific + 487
12  com.apple.CoreFoundation      	0x00007fff3e3186b3 CFRunLoopRun + 99
13  com.apple.WebCore             	0x0000000101c457dd WebCore::ScrollingThread::initializeRunLoop() + 253
14  com.apple.JavaScriptCore      	0x000000010623e694 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) + 212
15  com.apple.JavaScriptCore      	0x0000000106240959 WTF::wtfThreadEntryPoint(void*) + 9
16  libsystem_pthread.dylib       	0x00007fff665b2661 _pthread_body + 340
17  libsystem_pthread.dylib       	0x00007fff665b250d _pthread_start + 377
18  libsystem_pthread.dylib       	0x00007fff665b1bf9 thread_start + 13

[Chris Dumez]

Created attachment 375836 [details]
Patch

[Antti Koivisto]

Comment on attachment 375836 [details]
Patch

In which thread is it supposed to be deleted?

[Simon Fraser (smfr)]

Chris and I talked about this. We have an explicit hand-off of the state tree in ScrollingCoordinatorMac::commitTreeState() which may be tripping his assertions.

[Chris Dumez]

Created attachment 375837 [details]
Patch

[Chris Dumez]

(In reply to Simon Fraser (smfr) from comment #3)
> Chris and I talked about this. We have an explicit hand-off of the state
> tree in ScrollingCoordinatorMac::commitTreeState() which may be tripping his
> assertions.

(In reply to Antti Koivisto from comment #2)
> Comment on attachment 375836 [details]
> Patch
> 
> In which thread is it supposed to be deleted?

I am not familiar enough with this part of the code to be sure, but looking at the ScrollingStateNode data members, I do not see anything obviously unsafe if constructing them on the main thread and then destroying them on the scrolling thread.

[Chris Dumez]

Created attachment 375838 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 375838 [details]
Patch

Clearing flags on attachment: 375838

Committed r248445: <https://trac.webkit.org/changeset/248445>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/54098566>