From: cdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Mon, 12 Aug 2019 17:27:51 +0000 (+0000)
Subject: GPUBuffer seems to be ref'd / deref'd from multiple thread concurrently but is not... 
X-Git-Url: http://git.webkit.org/?p=WebKit-https.git;a=commitdiff_plain;h=e240915d885d1e8915810f3bda857db006f94ad1

GPUBuffer seems to be ref'd / deref'd from multiple thread concurrently but is not ThreadSafeRefCounted
https://bugs.webkit.org/show_bug.cgi?id=200629

Reviewed by Geoffrey Garen.

Make sure GPUBuffer only gets ref'd / deref'd on the main thread, since it is not
ThreadSafeRefCounted.

* platform/graphics/gpu/cocoa/GPUBufferMetal.mm:
(WebCore::GPUBuffer::commandBufferCommitted):
(WebCore::GPUBuffer::commandBufferCompleted):


git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248532 268f45cc-cd09-0410-ab3c-d52691b4dbfc
---

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index ed9d8526..e749160 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,17 @@
+2019-08-12  Chris Dumez  <cdumez@apple.com>
+
+        GPUBuffer seems to be ref'd / deref'd from multiple thread concurrently but is not ThreadSafeRefCounted
+        https://bugs.webkit.org/show_bug.cgi?id=200629
+
+        Reviewed by Geoffrey Garen.
+
+        Make sure GPUBuffer only gets ref'd / deref'd on the main thread, since it is not
+        ThreadSafeRefCounted.
+
+        * platform/graphics/gpu/cocoa/GPUBufferMetal.mm:
+        (WebCore::GPUBuffer::commandBufferCommitted):
+        (WebCore::GPUBuffer::commandBufferCompleted):
+
 2019-08-12  Thibault Saunier  <tsaunier@igalia.com>
 
         [GStreamer][WebRTC] Handle broken data in the libwebrtc GStreamer decoders
diff --git a/Source/WebCore/platform/graphics/gpu/cocoa/GPUBufferMetal.mm b/Source/WebCore/platform/graphics/gpu/cocoa/GPUBufferMetal.mm
index 2b277dc..f80eadf 100644
--- a/Source/WebCore/platform/graphics/gpu/cocoa/GPUBufferMetal.mm
+++ b/Source/WebCore/platform/graphics/gpu/cocoa/GPUBufferMetal.mm
@@ -34,6 +34,7 @@
 #import <JavaScriptCore/ArrayBuffer.h>
 #import <Metal/Metal.h>
 #import <wtf/BlockObjCExceptions.h>
+#import <wtf/BlockPtr.h>
 #import <wtf/CheckedArithmetic.h>
 #import <wtf/MainThread.h>
 
@@ -148,25 +149,26 @@ JSC::ArrayBuffer* GPUBuffer::mapOnCreation()
 #if USE(METAL)
 void GPUBuffer::commandBufferCommitted(MTLCommandBuffer *commandBuffer)
 {
+    ASSERT(isMainThread());
     ++m_numScheduledCommandBuffers;
 
-    auto protectedThis = makeRefPtr(this);
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
-    [commandBuffer addCompletedHandler:^(id<MTLCommandBuffer>) {
-        protectedThis->commandBufferCompleted();
-    }];
+    // Make sure |this| only gets ref'd / deref'd on the main thread since it is not ThreadSafeRefCounted.
+    [commandBuffer addCompletedHandler:makeBlockPtr([this, protectedThis = makeRef(*this)](id<MTLCommandBuffer>) mutable {
+        callOnMainThread([this, protectedThis = WTFMove(protectedThis)] {
+            commandBufferCompleted();
+        });
+    }).get()];
     END_BLOCK_OBJC_EXCEPTIONS;
 }
 
 void GPUBuffer::commandBufferCompleted()
 {
+    ASSERT(isMainThread());
     ASSERT(m_numScheduledCommandBuffers);
 
-    if (m_numScheduledCommandBuffers == 1 && state() == State::Mapped) {
-        callOnMainThread([this, protectedThis = makeRef(*this)] () {
-            runMappingCallback();
-        });
-    }
+    if (m_numScheduledCommandBuffers == 1 && state() == State::Mapped)
+        runMappingCallback();
 
     --m_numScheduledCommandBuffers;
 }