diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index abf8d6038b..6e41e56414 100644 --- a/src/dawn_native/Device.cpp +++ b/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 diff --git a/src/dawn_native/Fence.cpp b/src/dawn_native/Fence.cpp index 1ad89b9593..f16274e9d3 100644 --- a/src/dawn_native/Fence.cpp +++ b/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))) { - callback(WGPUFenceCompletionStatus_Error, userdata); + WGPUFenceCompletionStatus status; + 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)) { - request.completionCallback(WGPUFenceCompletionStatus_Success, request.userdata); + 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 {}; } diff --git a/src/dawn_native/Fence.h b/src/dawn_native/Fence.h index 1211ecbf8f..a9845f9cb0 100644 --- a/src/dawn_native/Fence.h +++ b/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; diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index 4492f53e19..1876f9bce4 100644 --- a/src/dawn_native/Queue.cpp +++ b/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)); diff --git a/src/tests/end2end/DeviceLostTests.cpp b/src/tests/end2end/DeviceLostTests.cpp index 103d6f4613..f62c7ad16c 100644 --- a/src/tests/end2end/DeviceLostTests.cpp +++ b/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; +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(); + mockFenceOnCompletionCallback = std::make_unique(); } void TearDown() override { @@ -55,20 +74,11 @@ class DeviceLostTest : public DawnTest { device.LoseForTesting(); } - static void CheckMapWriteFail(WGPUBufferMapAsyncStatus status, - void* data, - uint64_t datalength, - 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) { + template + static void MapFailCallback(WGPUBufferMapAsyncStatus status, + T* data, + uint64_t datalength, + void* userdata) { EXPECT_EQ(WGPUBufferMapAsyncStatus_DeviceLost, status); EXPECT_EQ(nullptr, data); EXPECT_EQ(0u, datalength); @@ -252,7 +262,7 @@ TEST_P(DeviceLostTest, BufferMapWriteAsyncFails) { wgpu::Buffer buffer = device.CreateBuffer(&bufferDescriptor); SetCallbackAndLoseForTesting(); - ASSERT_DEVICE_ERROR(buffer.MapWriteAsync(CheckMapWriteFail, const_cast(&fakeUserData))); + ASSERT_DEVICE_ERROR(buffer.MapWriteAsync(MapFailCallback, const_cast(&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(&fakeUserData)); + buffer.MapWriteAsync(MapFailCallback, const_cast(&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(&fakeUserData))); + ASSERT_DEVICE_ERROR(buffer.MapReadAsync(MapFailCallback, const_cast(&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(&fakeUserData)); + buffer.MapReadAsync(MapFailCallback, const_cast(&fakeUserData)); SetCallbackAndLoseForTesting(); } @@ -366,4 +376,89 @@ TEST_P(DeviceLostTest, CommandEncoderFinishFails) { ASSERT_DEVICE_ERROR(encoder.Finish()); } -DAWN_INSTANTIATE_TEST(DeviceLostTest, D3D12Backend, VulkanBackend); \ No newline at end of file +// 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);