Bug 200629

Summary: GPUBuffer seems to be ref'd / deref'd from multiple thread concurrently but is not ThreadSafeRefCounted
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, commit-queue, dino, ews-watchlist, ggaren, graouts, justin_fan, kondapallykalyan, mmaxfield, thorton, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 200507    
Attachments:
Description Flags
Patch none

Chris Dumez
Reported 2019-08-12 08:50:23 PDT
GPUBuffer seems to be ref'd / deref'd from multiple thread concurrently but is not ThreadSafeRefCounted: stderr: 2 0x21a25e471 WTF::RefCountedBase::derefBase() const 3 0x21aa4aa2f WTF::RefCounted<WebCore::GPUObjectBase, std::__1::default_delete<WebCore::GPUObjectBase> >::deref() const 4 0x21aa8c451 void WTF::derefIfNotNull<WebCore::GPUBuffer>(WebCore::GPUBuffer*) 5 0x21aa8c419 WTF::RefPtr<WebCore::GPUBuffer, WTF::DumbPtrTraits<WebCore::GPUBuffer> >::~RefPtr() 6 0x21aa79695 WTF::RefPtr<WebCore::GPUBuffer, WTF::DumbPtrTraits<WebCore::GPUBuffer> >::~RefPtr() 7 0x21aa79679 __destroy_helper_block_e8_32c63_ZTSN3WTF6RefPtrIN7WebCore9GPUBufferENS_13DumbPtrTraitsIS2_EEEE 8 0x7fff737a7bf3 _Block_release 9 0x7fff4c38571a _doMTLDispatch 10 0x7fff4c385c04 -[_MTLCommandBuffer didCompleteWithStartTime:endTime:error:] 11 0x7fff4c3859fe -[MTLIOAccelCommandBuffer didCompleteWithStartTime:endTime:error:] 12 0x7fff4c3858ed -[_MTLCommandQueue commandBufferDidComplete:startTime:completionTime:error:] 13 0x7fff64b786d2 ioAccelCommandQueueBlockFenceCallback 14 0x7fff4a10f7ab IODispatchCalloutFromCFMessage 15 0x7fff4a10f62d _IODispatchCalloutWithDispatch 16 0x7fff73727bde dispatch_mig_server 17 0x7fff7371163d _dispatch_client_callout 18 0x7fff73713de6 _dispatch_continuation_pop 19 0x7fff73722f42 _dispatch_source_invoke 20 0x7fff73717792 _dispatch_lane_serial_drain 21 0x7fff73718396 _dispatch_lane_invoke 22 0x7fff737206ed _dispatch_workloop_worker_thread 23 0x7fff73951611 _pthread_wqthread 24 0x7fff739513fd start_wqthread LEAK: 1 WebPageProxy
Attachments
Patch (2.72 KB, patch)
2019-08-12 09:25 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2019-08-12 08:59:40 PDT
GPUBuffer gets deref'd on a background Metal thread even though the main threads holds references to it: Thread 11 Crashed:: Dispatch queue: com.Metal.CompletionQueueDispatch 0 com.apple.JavaScriptCore 0x0000000777d472be WTFCrash + 14 (Assertions.cpp:305) 1 com.apple.WebCore 0x0000000760017271 WTF::RefCountedBase::derefBase() const + 209 (RefCounted.h:136) 2 com.apple.WebCore 0x000000076079651f WTF::RefCounted<WebCore::GPUObjectBase, std::__1::default_delete<WebCore::GPUObjectBase> >::deref() const + 31 (RefCounted.h:196) 3 com.apple.WebCore 0x00000007607d50f1 void WTF::derefIfNotNull<WebCore::GPUBuffer>(WebCore::GPUBuffer*) + 49 (RefPtr.h:45) 4 com.apple.WebCore 0x00000007607d50b9 WTF::RefPtr<WebCore::GPUBuffer, WTF::DumbPtrTraits<WebCore::GPUBuffer> >::~RefPtr() + 41 (RefPtr.h:69) 5 com.apple.WebCore 0x00000007607c3395 WTF::RefPtr<WebCore::GPUBuffer, WTF::DumbPtrTraits<WebCore::GPUBuffer> >::~RefPtr() + 21 (RefPtr.h:69) 6 com.apple.WebCore 0x00000007607c3379 __destroy_helper_block_e8_32c63_ZTSN3WTF6RefPtrIN7WebCore9GPUBufferENS_13DumbPtrTraitsIS2_EEEE + 25 (GPUBufferMetal.mm:155) 7 libsystem_blocks.dylib 0x00007fff6e46ebed _Block_release + 101 8 com.apple.Metal 0x00007fff3c80dc14 _doMTLDispatch + 43 9 com.apple.Metal 0x00007fff3c80dfa1 -[_MTLCommandBuffer didCompleteWithStartTime:endTime:error:] + 378 10 com.apple.Metal 0x00007fff3c80de06 -[MTLIOAccelCommandBuffer didCompleteWithStartTime:endTime:error:] + 88 11 com.apple.Metal 0x00007fff3c80dcf5 -[_MTLCommandQueue commandBufferDidComplete:startTime:completionTime:error:] + 197 12 com.apple.IOAccelerator 0x00007fff5af0db89 ioAccelCommandQueueBlockFenceCallback + 44 13 com.apple.framework.IOKit 0x00007fff39e3c72b IODispatchCalloutFromCFMessage + 364 14 com.apple.framework.IOKit 0x00007fff39e3c5ac _IODispatchCalloutWithDispatch + 33 15 libdispatch.dylib 0x00007fff6e3de367 dispatch_mig_server + 356 16 libdispatch.dylib 0x00007fff6e3c75ee _dispatch_client_callout + 8 17 libdispatch.dylib 0x00007fff6e3c97a0 _dispatch_continuation_pop + 414 18 libdispatch.dylib 0x00007fff6e3d9440 _dispatch_source_invoke + 2084 19 libdispatch.dylib 0x00007fff6e3cca60 _dispatch_lane_serial_drain + 263 20 libdispatch.dylib 0x00007fff6e3cd532 _dispatch_lane_invoke + 363 21 libdispatch.dylib 0x00007fff6e3d6ba1 _dispatch_workloop_worker_thread + 582 22 libsystem_pthread.dylib 0x00007fff6e620773 _pthread_wqthread + 290 23 libsystem_pthread.dylib 0x00007fff6e6205cf start_wqthread + 15
Chris Dumez
Comment 2 2019-08-12 09:18:40 PDT
This code does not look safe: #if USE(METAL) void GPUBuffer::commandBufferCommitted(MTLCommandBuffer *commandBuffer) { ++m_numScheduledCommandBuffers; auto protectedThis = makeRefPtr(this); BEGIN_BLOCK_OBJC_EXCEPTIONS; [commandBuffer addCompletedHandler:^(id<MTLCommandBuffer>) { protectedThis->commandBufferCompleted(); }]; END_BLOCK_OBJC_EXCEPTIONS; } void GPUBuffer::commandBufferCompleted() { ASSERT(m_numScheduledCommandBuffers); if (m_numScheduledCommandBuffers == 1 && state() == State::Mapped) { callOnMainThread([this, protectedThis = makeRef(*this)] () { runMappingCallback(); }); } --m_numScheduledCommandBuffers; } #endif // USE(METAL) given that this involves multiple threads, refs/derefs |this| and |this| is a GPUBuffer which subclasses RefCounted.
Chris Dumez
Comment 3 2019-08-12 09:25:26 PDT
Created attachment 376067 [details] Patch
Geoffrey Garen
Comment 4 2019-08-12 09:30:20 PDT
Comment on attachment 376067 [details] Patch r=me
WebKit Commit Bot
Comment 5 2019-08-12 10:27:57 PDT
Comment on attachment 376067 [details] Patch Clearing flags on attachment: 376067 Committed r248532: <https://trac.webkit.org/changeset/248532>
WebKit Commit Bot
Comment 6 2019-08-12 10:27:58 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7 2019-08-12 10:28:26 PDT
<rdar://problem/54214916>
Note You need to log in before you can comment on or make changes to this bug.