Immediately call fence and map callbacks on device loss.
This is more in line with what happens in dawn_wire and Blink's WebGPU
implementation. It also allows fixing the Fence-related DeviceLost tests
to destroy the mock fence callback on destruction, which in turns fixes
a crash on dawn_end2end_tests exit on MSVC x64 debug.
Bug: dawn:602
Change-Id: I277e7fa284a573854ed46576602d5f6819db1357
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/38526
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp
index 74d74eb..4f89724 100644
--- a/src/dawn_native/Buffer.cpp
+++ b/src/dawn_native/Buffer.cpp
@@ -35,7 +35,10 @@
: buffer(std::move(buffer)), id(id) {
}
void Finish() override {
- buffer->OnMapRequestCompleted(id);
+ buffer->OnMapRequestCompleted(id, WGPUBufferMapAsyncStatus_Success);
+ }
+ void HandleDeviceLoss() override {
+ buffer->OnMapRequestCompleted(id, WGPUBufferMapAsyncStatus_DeviceLost);
}
~MapRequestTask() override = default;
@@ -537,8 +540,8 @@
mState = BufferState::Destroyed;
}
- void BufferBase::OnMapRequestCompleted(MapRequestID mapID) {
- CallMapCallback(mapID, WGPUBufferMapAsyncStatus_Success);
+ void BufferBase::OnMapRequestCompleted(MapRequestID mapID, WGPUBufferMapAsyncStatus status) {
+ CallMapCallback(mapID, status);
}
bool BufferBase::IsDataInitialized() const {
diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h
index 675b071..936ae3c 100644
--- a/src/dawn_native/Buffer.h
+++ b/src/dawn_native/Buffer.h
@@ -53,7 +53,7 @@
wgpu::BufferUsage GetUsage() const;
MaybeError MapAtCreation();
- void OnMapRequestCompleted(MapRequestID mapID);
+ void OnMapRequestCompleted(MapRequestID mapID, WGPUBufferMapAsyncStatus status);
MaybeError ValidateCanUseOnQueueNow() const;
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 9a2b862..78a1050 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -240,6 +240,8 @@
// The device was lost, call the application callback.
if (type == InternalErrorType::DeviceLost && mDeviceLostCallback != nullptr) {
+ mDefaultQueue->HandleDeviceLoss();
+
mDeviceLostCallback(message, mDeviceLostUserdata);
mDeviceLostCallback = nullptr;
}
diff --git a/src/dawn_native/Fence.cpp b/src/dawn_native/Fence.cpp
index c1ce24b..8069149 100644
--- a/src/dawn_native/Fence.cpp
+++ b/src/dawn_native/Fence.cpp
@@ -28,7 +28,10 @@
: fence(std::move(fence)), value(value) {
}
void Finish() override {
- fence->SetCompletedValue(value);
+ fence->SetCompletedValue(value, WGPUFenceCompletionStatus_Success);
+ }
+ void HandleDeviceLoss() override {
+ fence->SetCompletedValue(value, WGPUFenceCompletionStatus_DeviceLost);
}
~FenceInFlight() override = default;
@@ -116,18 +119,14 @@
mSignalValue = signalValue;
}
- void Fence::SetCompletedValue(FenceAPISerial completedValue) {
+ void Fence::SetCompletedValue(FenceAPISerial completedValue, WGPUFenceCompletionStatus status) {
ASSERT(!IsError());
ASSERT(completedValue <= mSignalValue);
ASSERT(completedValue > mCompletedValue);
mCompletedValue = completedValue;
for (auto& request : mRequests.IterateUpTo(mCompletedValue)) {
- if (GetDevice()->IsLost()) {
- request.completionCallback(WGPUFenceCompletionStatus_DeviceLost, request.userdata);
- } else {
- request.completionCallback(WGPUFenceCompletionStatus_Success, request.userdata);
- }
+ request.completionCallback(status, request.userdata);
}
mRequests.ClearUpTo(mCompletedValue);
}
diff --git a/src/dawn_native/Fence.h b/src/dawn_native/Fence.h
index 6c1cbcb..9bc471e 100644
--- a/src/dawn_native/Fence.h
+++ b/src/dawn_native/Fence.h
@@ -46,7 +46,7 @@
friend class QueueBase;
friend struct FenceInFlight;
void SetSignaledValue(FenceAPISerial signalValue);
- void SetCompletedValue(FenceAPISerial completedValue);
+ void SetCompletedValue(FenceAPISerial completedValue, WGPUFenceCompletionStatus status);
void UpdateFenceOnComplete(Fence* fence, FenceAPISerial value);
private:
diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp
index 7a65431..1bb56de 100644
--- a/src/dawn_native/Queue.cpp
+++ b/src/dawn_native/Queue.cpp
@@ -192,6 +192,13 @@
mTasksInFlight.ClearUpTo(finishedSerial);
}
+ void QueueBase::HandleDeviceLoss() {
+ for (auto& task : mTasksInFlight.IterateAll()) {
+ task->HandleDeviceLoss();
+ }
+ mTasksInFlight.Clear();
+ }
+
Fence* QueueBase::CreateFence(const FenceDescriptor* descriptor) {
if (GetDevice()->ConsumedError(ValidateCreateFence(descriptor))) {
return Fence::MakeError(GetDevice());
diff --git a/src/dawn_native/Queue.h b/src/dawn_native/Queue.h
index 59e3117..a36d912 100644
--- a/src/dawn_native/Queue.h
+++ b/src/dawn_native/Queue.h
@@ -30,6 +30,7 @@
struct TaskInFlight {
virtual ~TaskInFlight();
virtual void Finish() = 0;
+ virtual void HandleDeviceLoss() = 0;
};
static QueueBase* MakeError(DeviceBase* device);
@@ -52,6 +53,7 @@
void TrackTask(std::unique_ptr<TaskInFlight> task, ExecutionSerial serial);
void Tick(ExecutionSerial finishedSerial);
+ void HandleDeviceLoss();
protected:
QueueBase(DeviceBase* device);
diff --git a/src/tests/end2end/DeviceLostTests.cpp b/src/tests/end2end/DeviceLostTests.cpp
index 31251a0..1295288 100644
--- a/src/tests/end2end/DeviceLostTests.cpp
+++ b/src/tests/end2end/DeviceLostTests.cpp
@@ -65,6 +65,7 @@
void TearDown() override {
mockDeviceLostCallback = nullptr;
+ mockFenceOnCompletionCallback = nullptr;
DawnTest::TearDown();
}
@@ -449,8 +450,8 @@
ASSERT_DEVICE_ERROR(fence.OnCompletion(2u, ToMockFenceOnCompletionFails, nullptr));
ASSERT_DEVICE_ERROR(device.Tick());
- // completed value should not have changed from initial value
- EXPECT_EQ(fence.GetCompletedValue(), 0u);
+ // completed value is the last value signaled (all previous GPU operations are as if completed)
+ EXPECT_EQ(fence.GetCompletedValue(), 2u);
}
// Test that Fence::OnCompletion callbacks with device lost status when device is lost after calling
@@ -469,7 +470,7 @@
SetCallbackAndLoseForTesting();
ASSERT_DEVICE_ERROR(device.Tick());
- EXPECT_EQ(fence.GetCompletedValue(), 0u);
+ EXPECT_EQ(fence.GetCompletedValue(), 2u);
}
// Regression test for the Null backend not properly setting the completedSerial when
