200629 2019-08-12 08:50:23 -0700 GPUBuffer seems to be ref'd / deref'd from multiple thread concurrently but is not ThreadSafeRefCounted 2019-08-12 10:28:26 -0700 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 200507 1 cdumez cdumez achristensen commit-queue dino ews-watchlist ggaren graouts justin_fan kondapallykalyan mmaxfield thorton tsavell webkit-bug-importer oldest_to_newest 1560405 0 cdumez 2019-08-12 08:50:23 -0700 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 1560409 1 cdumez 2019-08-12 08:59:40 -0700 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 1560418 2 cdumez 2019-08-12 09:18:40 -0700 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. 1560421 3 376067 cdumez 2019-08-12 09:25:26 -0700 Created attachment 376067 Patch 1560424 4 376067 ggaren 2019-08-12 09:30:20 -0700 Comment on attachment 376067 Patch r=me 1560456 5 376067 commit-queue 2019-08-12 10:27:57 -0700 Comment on attachment 376067 Patch Clearing flags on attachment: 376067 Committed r248532: <https://trac.webkit.org/changeset/248532> 1560457 6 commit-queue 2019-08-12 10:27:58 -0700 All reviewed patches have been landed. Closing bug. 1560459 7 webkit-bug-importer 2019-08-12 10:28:26 -0700 <rdar://problem/54214916> 376067 2019-08-12 09:25:26 -0700 2019-08-12 10:27:57 -0700 Patch bug-200629-20190812092526.patch text/plain 2789 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ4NTI3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNDg2NWNlZmM2N2EzZjAy YmE5OGJhZWU2ZjIxNjcyNzFmZjIwNGM5MC4uY2NkZmQxZGVmNjNhNDhkYTUwYTFjMzlmN2NlODJh MmQ2OTkwMDZiMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE5LTA4LTEyICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgR1BVQnVmZmVyIHNlZW1zIHRv IGJlIHJlZidkIC8gZGVyZWYnZCBmcm9tIG11bHRpcGxlIHRocmVhZCBjb25jdXJyZW50bHkgYnV0 IGlzIG5vdCBUaHJlYWRTYWZlUmVmQ291bnRlZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0 Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjAwNjI5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZ IChPT1BTISkuCisKKyAgICAgICAgTWFrZSBzdXJlIEdQVUJ1ZmZlciBvbmx5IGdldHMgcmVmJ2Qg LyBkZXJlZidkIG9uIHRoZSBtYWluIHRocmVhZCwgc2luY2UgaXQgaXMgbm90CisgICAgICAgIFRo cmVhZFNhZmVSZWZDb3VudGVkLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvZ3B1L2Nv Y29hL0dQVUJ1ZmZlck1ldGFsLm1tOgorICAgICAgICAoV2ViQ29yZTo6R1BVQnVmZmVyOjpjb21t YW5kQnVmZmVyQ29tbWl0dGVkKToKKyAgICAgICAgKFdlYkNvcmU6OkdQVUJ1ZmZlcjo6Y29tbWFu ZEJ1ZmZlckNvbXBsZXRlZCk6CisKIDIwMTktMDgtMTIgIFlvdWVubiBGYWJsZXQgIDx5b3Vlbm5A YXBwbGUuY29tPgogCiAgICAgICAgIFJlbW92ZSBJREJWYWx1ZTo6bV9zZXNzaW9uSUQKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dwdS9jb2NvYS9HUFVCdWZm ZXJNZXRhbC5tbSBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dwdS9jb2NvYS9H UFVCdWZmZXJNZXRhbC5tbQppbmRleCAyYjI3N2RjY2I2NDU5YWU1NjkxMzAzMDAzZWYzM2QyNjMz NjkxNmM1Li5mODBlYWRmNTVjOWExMTA4YTYwMDg4NjlmZjcwNjUxZTA2OWJiNDA5IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9ncHUvY29jb2EvR1BVQnVmZmVy TWV0YWwubW0KKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3B1L2NvY29h L0dQVUJ1ZmZlck1ldGFsLm1tCkBAIC0zNCw2ICszNCw3IEBACiAjaW1wb3J0IDxKYXZhU2NyaXB0 Q29yZS9BcnJheUJ1ZmZlci5oPgogI2ltcG9ydCA8TWV0YWwvTWV0YWwuaD4KICNpbXBvcnQgPHd0 Zi9CbG9ja09iakNFeGNlcHRpb25zLmg+CisjaW1wb3J0IDx3dGYvQmxvY2tQdHIuaD4KICNpbXBv cnQgPHd0Zi9DaGVja2VkQXJpdGhtZXRpYy5oPgogI2ltcG9ydCA8d3RmL01haW5UaHJlYWQuaD4K IApAQCAtMTQ4LDI1ICsxNDksMjYgQEAgSlNDOjpBcnJheUJ1ZmZlciogR1BVQnVmZmVyOjptYXBP bkNyZWF0aW9uKCkKICNpZiBVU0UoTUVUQUwpCiB2b2lkIEdQVUJ1ZmZlcjo6Y29tbWFuZEJ1ZmZl ckNvbW1pdHRlZChNVExDb21tYW5kQnVmZmVyICpjb21tYW5kQnVmZmVyKQogeworICAgIEFTU0VS VChpc01haW5UaHJlYWQoKSk7CiAgICAgKyttX251bVNjaGVkdWxlZENvbW1hbmRCdWZmZXJzOwog Ci0gICAgYXV0byBwcm90ZWN0ZWRUaGlzID0gbWFrZVJlZlB0cih0aGlzKTsKICAgICBCRUdJTl9C TE9DS19PQkpDX0VYQ0VQVElPTlM7Ci0gICAgW2NvbW1hbmRCdWZmZXIgYWRkQ29tcGxldGVkSGFu ZGxlcjpeKGlkPE1UTENvbW1hbmRCdWZmZXI+KSB7Ci0gICAgICAgIHByb3RlY3RlZFRoaXMtPmNv bW1hbmRCdWZmZXJDb21wbGV0ZWQoKTsKLSAgICB9XTsKKyAgICAvLyBNYWtlIHN1cmUgfHRoaXN8 IG9ubHkgZ2V0cyByZWYnZCAvIGRlcmVmJ2Qgb24gdGhlIG1haW4gdGhyZWFkIHNpbmNlIGl0IGlz IG5vdCBUaHJlYWRTYWZlUmVmQ291bnRlZC4KKyAgICBbY29tbWFuZEJ1ZmZlciBhZGRDb21wbGV0 ZWRIYW5kbGVyOm1ha2VCbG9ja1B0cihbdGhpcywgcHJvdGVjdGVkVGhpcyA9IG1ha2VSZWYoKnRo aXMpXShpZDxNVExDb21tYW5kQnVmZmVyPikgbXV0YWJsZSB7CisgICAgICAgIGNhbGxPbk1haW5U aHJlYWQoW3RoaXMsIHByb3RlY3RlZFRoaXMgPSBXVEZNb3ZlKHByb3RlY3RlZFRoaXMpXSB7Cisg ICAgICAgICAgICBjb21tYW5kQnVmZmVyQ29tcGxldGVkKCk7CisgICAgICAgIH0pOworICAgIH0p LmdldCgpXTsKICAgICBFTkRfQkxPQ0tfT0JKQ19FWENFUFRJT05TOwogfQogCiB2b2lkIEdQVUJ1 ZmZlcjo6Y29tbWFuZEJ1ZmZlckNvbXBsZXRlZCgpCiB7CisgICAgQVNTRVJUKGlzTWFpblRocmVh ZCgpKTsKICAgICBBU1NFUlQobV9udW1TY2hlZHVsZWRDb21tYW5kQnVmZmVycyk7CiAKLSAgICBp ZiAobV9udW1TY2hlZHVsZWRDb21tYW5kQnVmZmVycyA9PSAxICYmIHN0YXRlKCkgPT0gU3RhdGU6 Ok1hcHBlZCkgewotICAgICAgICBjYWxsT25NYWluVGhyZWFkKFt0aGlzLCBwcm90ZWN0ZWRUaGlz ID0gbWFrZVJlZigqdGhpcyldICgpIHsKLSAgICAgICAgICAgIHJ1bk1hcHBpbmdDYWxsYmFjaygp OwotICAgICAgICB9KTsKLSAgICB9CisgICAgaWYgKG1fbnVtU2NoZWR1bGVkQ29tbWFuZEJ1ZmZl cnMgPT0gMSAmJiBzdGF0ZSgpID09IFN0YXRlOjpNYXBwZWQpCisgICAgICAgIHJ1bk1hcHBpbmdD YWxsYmFjaygpOwogCiAgICAgLS1tX251bVNjaGVkdWxlZENvbW1hbmRCdWZmZXJzOwogfQo=