Handle Device Lost for Fence and Queue::Signal

Bug: dawn:68
Change-Id: I5391d55f85fba7dce28b1df5bb06c2d9217dc72a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/15420
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/Device.cpp

@@ -97,6 +97,10 @@ namespace dawn_native {
 
     void DeviceBase::BaseDestructor() {
         if (mLossStatus != LossStatus::Alive) {
+            // if device is already lost, we may still have fences and error scopes to clear since
+            // the time the device was lost, clear them now before we destruct the device.
+            mErrorScopeTracker->Tick(GetCompletedCommandSerial());
+            mFenceSignalTracker->Tick(GetCompletedCommandSerial());
             return;
         }
         // Assert that errors are device loss so that we can continue with destruction

src/dawn_native/Fence.cpp

@@ -66,8 +66,9 @@ namespace dawn_native {
     void Fence::OnCompletion(uint64_t value,
                              wgpu::FenceOnCompletionCallback callback,
                              void* userdata) {
-        if (GetDevice()->ConsumedError(ValidateOnCompletion(value))) {
+        WGPUFenceCompletionStatus status;
-            callback(WGPUFenceCompletionStatus_Error, userdata);
+        if (GetDevice()->ConsumedError(ValidateOnCompletion(value, &status))) {
+            callback(status, userdata);
             return;
         }
         ASSERT(!IsError());
@@ -106,16 +107,28 @@ namespace dawn_native {
         mCompletedValue = completedValue;
 
         for (auto& request : mRequests.IterateUpTo(mCompletedValue)) {
+            if (GetDevice()->IsLost()) {
+                request.completionCallback(WGPUFenceCompletionStatus_DeviceLost, request.userdata);
+            } else {
                 request.completionCallback(WGPUFenceCompletionStatus_Success, request.userdata);
             }
+        }
         mRequests.ClearUpTo(mCompletedValue);
     }
 
-    MaybeError Fence::ValidateOnCompletion(uint64_t value) const {
+    MaybeError Fence::ValidateOnCompletion(uint64_t value,
+                                           WGPUFenceCompletionStatus* status) const {
+        *status = WGPUFenceCompletionStatus_DeviceLost;
+        DAWN_TRY(GetDevice()->ValidateIsAlive());
+
+        *status = WGPUFenceCompletionStatus_Error;
         DAWN_TRY(GetDevice()->ValidateObject(this));
+
         if (value > mSignalValue) {
             return DAWN_VALIDATION_ERROR("Value greater than fence signaled value");
         }
+
+        *status = WGPUFenceCompletionStatus_Success;
         return {};
     }
 

src/dawn_native/Fence.h

@@ -51,7 +51,7 @@ namespace dawn_native {
       private:
         Fence(DeviceBase* device, ObjectBase::ErrorTag tag);
 
-        MaybeError ValidateOnCompletion(uint64_t value) const;
+        MaybeError ValidateOnCompletion(uint64_t value, WGPUFenceCompletionStatus* status) const;
 
         struct OnCompletionData {
             wgpu::FenceOnCompletionCallback completionCallback = nullptr;

src/dawn_native/Queue.cpp

@@ -118,6 +118,7 @@ namespace dawn_native {
     }
 
     MaybeError QueueBase::ValidateSignal(const Fence* fence, uint64_t signalValue) {
+        DAWN_TRY(GetDevice()->ValidateIsAlive());
         DAWN_TRY(GetDevice()->ValidateObject(this));
         DAWN_TRY(GetDevice()->ValidateObject(fence));
 
@@ -132,6 +133,7 @@ namespace dawn_native {
     }
 
     MaybeError QueueBase::ValidateCreateFence(const FenceDescriptor* descriptor) {
+        DAWN_TRY(GetDevice()->ValidateIsAlive());
         DAWN_TRY(GetDevice()->ValidateObject(this));
         DAWN_TRY(ValidateFenceDescriptor(descriptor));
 

src/tests/end2end/DeviceLostTests.cpp

@@ -34,6 +34,24 @@ static void ToMockDeviceLostCallback(const char* message, void* userdata) {
     self->StartExpectDeviceError();
 }
 
+class MockFenceOnCompletionCallback {
+  public:
+    MOCK_METHOD2(Call, void(WGPUFenceCompletionStatus status, void* userdata));
+};
+
+static std::unique_ptr<MockFenceOnCompletionCallback> mockFenceOnCompletionCallback;
+static void ToMockFenceOnCompletionCallbackFails(WGPUFenceCompletionStatus status, void* userdata) {
+    EXPECT_EQ(WGPUFenceCompletionStatus_DeviceLost, status);
+    mockFenceOnCompletionCallback->Call(status, userdata);
+    mockFenceOnCompletionCallback = nullptr;
+}
+static void ToMockFenceOnCompletionCallbackSucceeds(WGPUFenceCompletionStatus status,
+                                                    void* userdata) {
+    EXPECT_EQ(WGPUFenceCompletionStatus_Success, status);
+    mockFenceOnCompletionCallback->Call(status, userdata);
+    mockFenceOnCompletionCallback = nullptr;
+}
+
 static const int fakeUserData = 0;
 
 class DeviceLostTest : public DawnTest {
@@ -42,6 +60,7 @@ class DeviceLostTest : public DawnTest {
         DAWN_SKIP_TEST_IF(UsesWire());
         DawnTest::TestSetUp();
         mockDeviceLostCallback = std::make_unique<MockDeviceLostCallback>();
+        mockFenceOnCompletionCallback = std::make_unique<MockFenceOnCompletionCallback>();
     }
 
     void TearDown() override {
@@ -55,18 +74,9 @@ class DeviceLostTest : public DawnTest {
         device.LoseForTesting();
     }
 
-    static void CheckMapWriteFail(WGPUBufferMapAsyncStatus status,
+    template <typename T>
-                                  void* data,
+    static void MapFailCallback(WGPUBufferMapAsyncStatus status,
-                                  uint64_t datalength,
+                                T* data,
-                                  void* userdata) {
-        EXPECT_EQ(WGPUBufferMapAsyncStatus_DeviceLost, status);
-        EXPECT_EQ(nullptr, data);
-        EXPECT_EQ(0u, datalength);
-        EXPECT_EQ(&fakeUserData, userdata);
-    }
-
-    static void CheckMapReadFail(WGPUBufferMapAsyncStatus status,
-                                 const void* data,
                                 uint64_t datalength,
                                 void* userdata) {
         EXPECT_EQ(WGPUBufferMapAsyncStatus_DeviceLost, status);
@@ -252,7 +262,7 @@ TEST_P(DeviceLostTest, BufferMapWriteAsyncFails) {
     wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor);
 
     SetCallbackAndLoseForTesting();
-    ASSERT_DEVICE_ERROR(buffer.MapWriteAsync(CheckMapWriteFail, const_cast<int*>(&fakeUserData)));
+    ASSERT_DEVICE_ERROR(buffer.MapWriteAsync(MapFailCallback, const_cast<int*>(&fakeUserData)));
 }
 
 // Test that buffer.MapWriteAsync calls back with device loss status
@@ -262,7 +272,7 @@ TEST_P(DeviceLostTest, BufferMapWriteAsyncBeforeLossFails) {
     bufferDescriptor.usage = wgpu::BufferUsage::MapWrite;
     wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor);
 
-    buffer.MapWriteAsync(CheckMapWriteFail, const_cast<int*>(&fakeUserData));
+    buffer.MapWriteAsync(MapFailCallback, const_cast<int*>(&fakeUserData));
     SetCallbackAndLoseForTesting();
 }
 
@@ -329,7 +339,7 @@ TEST_P(DeviceLostTest, BufferMapReadAsyncFails) {
     wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor);
 
     SetCallbackAndLoseForTesting();
-    ASSERT_DEVICE_ERROR(buffer.MapReadAsync(CheckMapReadFail, const_cast<int*>(&fakeUserData)));
+    ASSERT_DEVICE_ERROR(buffer.MapReadAsync(MapFailCallback, const_cast<int*>(&fakeUserData)));
 }
 
 // Test that BufferMapReadAsync calls back with device lost status when device lost after map read
@@ -340,7 +350,7 @@ TEST_P(DeviceLostTest, BufferMapReadAsyncBeforeLossFails) {
 
     wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor);
 
-    buffer.MapReadAsync(CheckMapReadFail, const_cast<int*>(&fakeUserData));
+    buffer.MapReadAsync(MapFailCallback, const_cast<int*>(&fakeUserData));
     SetCallbackAndLoseForTesting();
 }
 
@@ -366,4 +376,89 @@ TEST_P(DeviceLostTest, CommandEncoderFinishFails) {
     ASSERT_DEVICE_ERROR(encoder.Finish());
 }
 
+// Test that CreateFenceFails when device is lost
+TEST_P(DeviceLostTest, CreateFenceFails) {
+    SetCallbackAndLoseForTesting();
+
+    wgpu::FenceDescriptor descriptor;
+    descriptor.initialValue = 0;
+
+    ASSERT_DEVICE_ERROR(queue.CreateFence(&descriptor));
+}
+
+// Test that queue signal fails when device is lost
+TEST_P(DeviceLostTest, QueueSignalFenceFails) {
+    wgpu::FenceDescriptor descriptor;
+    descriptor.initialValue = 0;
+    wgpu::Fence fence = queue.CreateFence(&descriptor);
+
+    SetCallbackAndLoseForTesting();
+
+    ASSERT_DEVICE_ERROR(queue.Signal(fence, 3));
+
+    // callback should have device lost status
+    EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_DeviceLost, nullptr))
+        .Times(1);
+    ASSERT_DEVICE_ERROR(fence.OnCompletion(2u, ToMockFenceOnCompletionCallbackFails, nullptr));
+
+    // completed value should not have changed from initial value
+    EXPECT_EQ(fence.GetCompletedValue(), descriptor.initialValue);
+}
+
+// Test that Fence On Completion fails after device is lost
+TEST_P(DeviceLostTest, FenceOnCompletionFails) {
+    wgpu::FenceDescriptor descriptor;
+    descriptor.initialValue = 0;
+    wgpu::Fence fence = queue.CreateFence(&descriptor);
+
+    queue.Signal(fence, 2);
+
+    SetCallbackAndLoseForTesting();
+    // callback should have device lost status
+    EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_DeviceLost, nullptr))
+        .Times(1);
+    ASSERT_DEVICE_ERROR(fence.OnCompletion(2u, ToMockFenceOnCompletionCallbackFails, nullptr));
+    ASSERT_DEVICE_ERROR(device.Tick());
+
+    // completed value should not have changed from initial value
+    EXPECT_EQ(fence.GetCompletedValue(), 0u);
+}
+
+// Test that Fence::OnCompletion callbacks with device lost status when device is lost after calling
+// OnCompletion
+TEST_P(DeviceLostTest, FenceOnCompletionBeforeLossFails) {
+    wgpu::FenceDescriptor descriptor;
+    descriptor.initialValue = 0;
+    wgpu::Fence fence = queue.CreateFence(&descriptor);
+
+    queue.Signal(fence, 2);
+
+    // callback should have device lost status
+    EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_DeviceLost, nullptr))
+        .Times(1);
+    fence.OnCompletion(2u, ToMockFenceOnCompletionCallbackFails, nullptr);
+    SetCallbackAndLoseForTesting();
+    ASSERT_DEVICE_ERROR(device.Tick());
+
+    EXPECT_EQ(fence.GetCompletedValue(), 0u);
+}
+
+// Test that when you Signal, then Tick, then device lost, the fence completed value would be 2
+TEST_P(DeviceLostTest, FenceSignalTickOnCompletion) {
+    wgpu::FenceDescriptor descriptor;
+    descriptor.initialValue = 0;
+    wgpu::Fence fence = queue.CreateFence(&descriptor);
+
+    queue.Signal(fence, 2);
+    device.Tick();
+
+    // callback should have device lost status
+    EXPECT_CALL(*mockFenceOnCompletionCallback, Call(WGPUFenceCompletionStatus_Success, nullptr))
+        .Times(1);
+    fence.OnCompletion(2u, ToMockFenceOnCompletionCallbackSucceeds, nullptr);
+    SetCallbackAndLoseForTesting();
+
+    EXPECT_EQ(fence.GetCompletedValue(), 2u);
+}
+
 DAWN_INSTANTIATE_TEST(DeviceLostTest, D3D12Backend, VulkanBackend);