From 3cd31e647c7de96644e52613b3b23270736654fb Mon Sep 17 00:00:00 2001
From: Corentin Wallez <cwallez@chromium.org>
Date: Mon, 23 Nov 2020 16:16:00 +0000
Subject: [PATCH] Metal: Prevent data race on mLastSubmittedCommands

There was a data-race between getting and using the pointer to
mLastSubmittedCommands in WaitForCommandsToBeScheduled and setting it
inside the scheduling handler.

Lock the mutex before calling waitUntilScheduled so that the object
doesn't get destroyed from underneath us.

Bug: chromium:1142024
Change-Id: Iadbf473530342de6d108d39285d723815452ac7c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/31260
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
---
 src/dawn_native/metal/DeviceMTL.mm | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/dawn_native/metal/DeviceMTL.mm b/src/dawn_native/metal/DeviceMTL.mm
index 9bd4fea36b..3846d0d480 100644
--- a/src/dawn_native/metal/DeviceMTL.mm
+++ b/src/dawn_native/metal/DeviceMTL.mm
@@ -357,7 +357,16 @@ namespace dawn_native { namespace metal {
 
     void Device::WaitForCommandsToBeScheduled() {
         SubmitPendingCommandBuffer();
-        [*mLastSubmittedCommands waitUntilScheduled];
+
+        // Only lock the object while we take a reference to it, otherwise we could block further
+        // progress if the driver calls the scheduled handler (which also acquires the lock) before
+        // finishing the waitUntilScheduled.
+        NSPRef<id<MTLCommandBuffer>> lastSubmittedCommands;
+        {
+            std::lock_guard<std::mutex> lock(mLastSubmittedCommandsMutex);
+            lastSubmittedCommands = mLastSubmittedCommands;
+        }
+        [*lastSubmittedCommands waitUntilScheduled];
     }
 
     MaybeError Device::WaitForIdleForDestruction() {