Revert "[dawn][headers] Add GetLostFuture to native and wire."
This reverts commit c45ef7eb9c4a34299dcf284bab1367d8dcea8c42.
Reason for revert: Seeing ASAN issues after this CL landed. Reverting in an attempt to verify if this is the issue or not. See https://chromium-review.googlesource.com/c/chromium/src/+/6057187?tab=checks
Original change's description:
> [dawn][headers] Add GetLostFuture to native and wire.
>
> Bug: 377753478
> Change-Id: I1ca3fb594388b409ed46e396966830fce6c272ff
> Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/214157
> Reviewed-by: Kai Ninomiya <kainino@chromium.org>
> Commit-Queue: Loko Kung <lokokung@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: 377753478
Change-Id: If08e2f2127c1165b0c5acbce1afcf5cdf4b208c0
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/217654
Reviewed-by: dan sinclair <dsinclair@google.com>
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
diff --git a/src/dawn/dawn.json b/src/dawn/dawn.json
index 2f6e976..aa32f4d 100644
--- a/src/dawn/dawn.json
+++ b/src/dawn/dawn.json
@@ -1535,7 +1535,8 @@
},
{
"name": "get lost future",
- "returns": "future"
+ "returns": "future",
+ "tags": ["emscripten"]
},
{
"name": "has feature",
diff --git a/src/dawn/dawn_wire.json b/src/dawn/dawn_wire.json
index 97b23ea..240a9d0 100644
--- a/src/dawn/dawn_wire.json
+++ b/src/dawn/dawn_wire.json
@@ -246,7 +246,6 @@
"DeviceGetAdapterInfo",
"DeviceGetFeatures",
"DeviceGetLimits",
- "DeviceGetLostFuture",
"DeviceHasFeature",
"DevicePopErrorScope",
"DevicePopErrorScopeF",
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index bfc65ed..0641cc1 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -300,7 +300,6 @@
mNextPipelineCompatibilityToken(1) {
DAWN_ASSERT(descriptor);
- DAWN_ASSERT(mLostEvent);
mLostEvent->mDevice = this;
#if defined(DAWN_ENABLE_ASSERTS)
@@ -411,6 +410,7 @@
// We need to explicitly release the Queue before we complete the destructor so that the
// Queue does not get destroyed after the Device.
mQueue = nullptr;
+ mLostEvent = nullptr;
}
MaybeError DeviceBase::Initialize(Ref<QueueBase> defaultQueue) {
@@ -568,7 +568,12 @@
// Skip handling device facilities if they haven't even been created (or failed doing so)
if (mState != State::BeingCreated) {
// The device is being destroyed so it will be lost, call the application callback.
- HandleDeviceLost(wgpu::DeviceLostReason::Destroyed, "Device was destroyed.");
+ if (mLostEvent != nullptr) {
+ mLostEvent->mReason = wgpu::DeviceLostReason::Destroyed;
+ mLostEvent->mMessage = "Device was destroyed.";
+ GetInstance()->GetEventManager()->SetFutureReady(mLostEvent.Get());
+ mLostEvent = nullptr;
+ }
// Call all the callbacks immediately as the device is about to shut down.
// TODO(crbug.com/dawn/826): Cancel the tasks that are in flight if possible.
@@ -650,18 +655,9 @@
Destroy();
}
-void DeviceBase::HandleDeviceLost(wgpu::DeviceLostReason reason, std::string_view message) {
- // Always use the first message and reason for device lost.
- if (mLostEvent->mMessage.empty()) {
- mLostEvent->mReason = reason;
- mLostEvent->mMessage = message;
- GetInstance()->GetEventManager()->SetFutureReady(mLostEvent.Get());
- }
-}
-
void DeviceBase::HandleError(std::unique_ptr<ErrorData> error,
InternalErrorType additionalAllowedErrors,
- wgpu::DeviceLostReason lostReason) {
+ WGPUDeviceLostReason lostReason) {
AppendDebugLayerMessages(error.get());
InternalErrorType type = error->GetType();
@@ -717,7 +713,13 @@
// The device was lost, schedule the application callback's execution.
// Note: we don't invoke the callbacks directly here because it could cause re-entrances ->
// possible deadlock.
- HandleDeviceLost(lostReason, messageStr);
+ if (mLostEvent != nullptr) {
+ mLostEvent->mReason = FromAPI(lostReason);
+ mLostEvent->mMessage = messageStr;
+ GetInstance()->GetEventManager()->SetFutureReady(mLostEvent.Get());
+ mLostEvent = nullptr;
+ }
+
mQueue->HandleDeviceLoss();
// TODO(crbug.com/dawn/826): Cancel the tasks that are in flight if possible.
@@ -881,7 +883,7 @@
// Note that since we are passing None as the allowedErrors, an additional message will be
// appended noting that the error was unexpected. Since this call is for testing only it is not
// too important, but useful for users to understand where the extra message is coming from.
- HandleError(DAWN_INTERNAL_ERROR(std::string(message)), InternalErrorType::None, reason);
+ HandleError(DAWN_INTERNAL_ERROR(std::string(message)), InternalErrorType::None, ToAPI(reason));
}
DeviceBase::State DeviceBase::GetState() const {
@@ -1905,10 +1907,6 @@
return mAdapter->APIGetInfo(adapterInfo);
}
-Future DeviceBase::APIGetLostFuture() const {
- return mLostEvent->GetFuture();
-}
-
void DeviceBase::APIInjectError(wgpu::ErrorType type, StringView message) {
if (ConsumedError(ValidateErrorType(type))) {
return;
diff --git a/src/dawn/native/Device.h b/src/dawn/native/Device.h
index b2c145b..2b0708f 100644
--- a/src/dawn/native/Device.h
+++ b/src/dawn/native/Device.h
@@ -123,7 +123,7 @@
// users as the respective error rather than causing a device loss instead.
void HandleError(std::unique_ptr<ErrorData> error,
InternalErrorType additionalAllowedErrors = InternalErrorType::None,
- wgpu::DeviceLostReason lost_reason = wgpu::DeviceLostReason::Unknown);
+ WGPUDeviceLostReason lost_reason = WGPUDeviceLostReason_Unknown);
MaybeError ValidateObject(const ApiObjectBase* object) const;
@@ -291,7 +291,6 @@
void APIGetFeatures(wgpu::SupportedFeatures* features) const;
void APIGetFeatures(SupportedFeatures* features) const;
wgpu::Status APIGetAdapterInfo(AdapterInfo* adapterInfo) const;
- Future APIGetLostFuture() const;
void APIInjectError(wgpu::ErrorType type, StringView message);
bool APITick();
void APIValidateTextureDescriptor(const TextureDescriptor* desc);
@@ -529,7 +528,6 @@
// ErrorSink implementation
void ConsumeError(std::unique_ptr<ErrorData> error,
InternalErrorType additionalAllowedErrors = InternalErrorType::None) override;
- void HandleDeviceLost(wgpu::DeviceLostReason reason, std::string_view message);
bool HasPendingTasks();
bool IsDeviceIdle();
diff --git a/src/dawn/native/EventManager.cpp b/src/dawn/native/EventManager.cpp
index 95f556b..7a82ec6 100644
--- a/src/dawn/native/EventManager.cpp
+++ b/src/dawn/native/EventManager.cpp
@@ -541,10 +541,6 @@
DAWN_ASSERT(mCompleted);
}
-Future EventManager::TrackedEvent::GetFuture() const {
- return {mFutureID};
-}
-
const EventManager::TrackedEvent::CompletionData& EventManager::TrackedEvent::GetCompletionData()
const {
return mCompletionData;
diff --git a/src/dawn/native/EventManager.h b/src/dawn/native/EventManager.h
index b0984af..a53f47d 100644
--- a/src/dawn/native/EventManager.h
+++ b/src/dawn/native/EventManager.h
@@ -130,8 +130,7 @@
// EventCompletionType::Shutdown.
~TrackedEvent() override;
- Future GetFuture() const;
-
+ class WaitRef;
// Events may be one of two types:
// - A queue and the ExecutionSerial after which the event will be completed.
// Used for queue completion.
diff --git a/src/dawn/tests/end2end/MultithreadTests.cpp b/src/dawn/tests/end2end/MultithreadTests.cpp
index b8c3bb5..d388689 100644
--- a/src/dawn/tests/end2end/MultithreadTests.cpp
+++ b/src/dawn/tests/end2end/MultithreadTests.cpp
@@ -191,40 +191,6 @@
});
}
-// Test that waiting for a device lost after it's lost does not block.
-TEST_P(MultithreadTests, Device_WaitForDroppedAfterDropped) {
- auto future = device.GetLostFuture();
-
- LoseDeviceForTesting();
- EXPECT_EQ(GetInstance().WaitAny(future, 0), wgpu::WaitStatus::Success);
-
- EXPECT_EQ(future.id, device.GetLostFuture().id);
-}
-
-// Test that we can wait for a device lost on another thread.
-TEST_P(MultithreadTests, Device_WaitForDroppedInAnotherThread) {
- // TODO(crbug.com/dawn/1779): This test seems to cause flakiness in other sampling tests on
- // NVIDIA.
- DAWN_SUPPRESS_TEST_IF(IsD3D12() && IsNvidia());
-
- enum class Step {
- Begin,
- Waiting,
- };
-
- LockStep<Step> lockStep(Step::Begin);
- std::thread waitThread([&] {
- auto future = device.GetLostFuture();
- EXPECT_EQ(GetInstance().WaitAny(future, 0), wgpu::WaitStatus::TimedOut);
- lockStep.Signal(Step::Waiting);
- EXPECT_EQ(GetInstance().WaitAny(future, UINT64_MAX), wgpu::WaitStatus::Success);
- });
-
- lockStep.Wait(Step::Waiting);
- LoseDeviceForTesting();
- waitThread.join();
-}
-
// Test that multiple buffers being created and mapped on multiple threads won't interfere with
// each other.
TEST_P(MultithreadTests, Buffers_MapInParallel) {
diff --git a/src/dawn/wire/client/Adapter.cpp b/src/dawn/wire/client/Adapter.cpp
index 2302b6d..2358d6c 100644
--- a/src/dawn/wire/client/Adapter.cpp
+++ b/src/dawn/wire/client/Adapter.cpp
@@ -298,7 +298,7 @@
cmd.eventManagerHandle = GetEventManagerHandle();
cmd.future = {futureIDInternal};
cmd.deviceObjectHandle = device->GetWireHandle();
- cmd.deviceLostFuture = device->GetLostFuture();
+ cmd.deviceLostFuture = device->GetDeviceLostFuture();
cmd.descriptor = &wireDescriptor;
cmd.userdataCount = 1;
@@ -330,7 +330,7 @@
cmd.eventManagerHandle = GetEventManagerHandle();
cmd.future = {futureIDInternal};
cmd.deviceObjectHandle = device->GetWireHandle();
- cmd.deviceLostFuture = device->GetLostFuture();
+ cmd.deviceLostFuture = device->GetDeviceLostFuture();
cmd.descriptor = &wireDescriptor;
cmd.userdataCount = 2;
diff --git a/src/dawn/wire/client/Device.cpp b/src/dawn/wire/client/Device.cpp
index 4677663..927dcc9 100644
--- a/src/dawn/wire/client/Device.cpp
+++ b/src/dawn/wire/client/Device.cpp
@@ -172,10 +172,9 @@
EventType GetType() override { return kType; }
WireResult ReadyHook(FutureID futureID, WGPUDeviceLostReason reason, WGPUStringView message) {
- if (mMessage.empty()) {
- mReason = reason;
- mMessage = ToString(message);
- }
+ mReason = reason;
+ mMessage = ToString(message);
+ mDevice->mDeviceLostInfo.futureID = kNullFutureID;
return WireResult::Success;
}
@@ -319,18 +318,22 @@
}
void Device::HandleDeviceLost(WGPUDeviceLostReason reason, WGPUStringView message) {
- FutureID futureID = GetLostFuture().id;
- DAWN_CHECK(GetEventManager().SetFutureReady<DeviceLostEvent>(futureID, reason, message) ==
- WireResult::Success);
+ FutureID futureID = GetDeviceLostFuture().id;
+ if (futureID != kNullFutureID) {
+ DAWN_CHECK(GetEventManager().SetFutureReady<DeviceLostEvent>(futureID, reason, message) ==
+ WireResult::Success);
+ }
mIsAlive = false;
}
-WGPUFuture Device::GetLostFuture() {
+WGPUFuture Device::GetDeviceLostFuture() {
// Lazily track the device lost event so that event ordering w.r.t RequestDevice is correct.
if (mDeviceLostInfo.event != nullptr) {
- auto [deviceLostFutureIDInternal, _] =
+ auto [deviceLostFutureIDInternal, tracked] =
GetEventManager().TrackEvent(std::move(mDeviceLostInfo.event));
- mDeviceLostInfo.futureID = deviceLostFutureIDInternal;
+ if (tracked) {
+ mDeviceLostInfo.futureID = deviceLostFutureIDInternal;
+ }
}
return {mDeviceLostInfo.futureID};
}
diff --git a/src/dawn/wire/client/Device.h b/src/dawn/wire/client/Device.h
index 5f7a1a4..9d7ae58 100644
--- a/src/dawn/wire/client/Device.h
+++ b/src/dawn/wire/client/Device.h
@@ -58,6 +58,7 @@
void SetFeatures(const WGPUFeatureName* features, uint32_t featuresCount);
bool IsAlive() const;
+ WGPUFuture GetDeviceLostFuture();
void HandleError(WGPUErrorType errorType, WGPUStringView message);
void HandleLogging(WGPULoggingType loggingType, WGPUStringView message);
@@ -93,7 +94,6 @@
const WGPUCreateRenderPipelineAsyncCallbackInfo2& callbackInfo);
WGPUStatus GetLimits(WGPUSupportedLimits* limits) const;
- WGPUFuture GetLostFuture();
bool HasFeature(WGPUFeatureName feature) const;
void GetFeatures(WGPUSupportedFeatures* features) const;
WGPUStatus GetAdapterInfo(WGPUAdapterInfo* info) const;
