[WGPUFuture] Manually scope the device lock for Future entry points.
Bug: dawn:2449 dawn:2451
Change-Id: I3cb02439c1392b7d521b5b5756facb7ae63f3edc
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/177933
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Loko Kung <lokokung@google.com>
Reviewed-by: Austin Eng <enga@chromium.org>
diff --git a/generator/templates/dawn/native/ProcTable.cpp b/generator/templates/dawn/native/ProcTable.cpp
index 7c7931c..c3ac507 100644
--- a/generator/templates/dawn/native/ProcTable.cpp
+++ b/generator/templates/dawn/native/ProcTable.cpp
@@ -71,7 +71,7 @@
{% endif %}
{%- endfor-%}
- {% if method.autolock %}
+ {% if method.autolock and method.return_type.name.get() != 'future' %}
{% if type.name.get() != "device" %}
auto device = self->GetDevice();
{% else %}
@@ -79,7 +79,7 @@
{% endif %}
auto deviceLock(device->GetScopedLock());
{% else %}
- // This method is specified to not use AutoLock in json script.
+ // This method is specified to not use AutoLock in json script or it returns a future.
{% endif %}
{% if method.return_type.name.canonical_case() != "void" %}
diff --git a/src/dawn/dawn.json b/src/dawn/dawn.json
index fe7c864..f158f38 100644
--- a/src/dawn/dawn.json
+++ b/src/dawn/dawn.json
@@ -1443,6 +1443,7 @@
},
{
"name": "pop error scope",
+ "no autolock": true,
"args": [
{"name": "old callback", "type": "error callback"},
{"name": "userdata", "type": "void *"}
diff --git a/src/dawn/native/Buffer.cpp b/src/dawn/native/Buffer.cpp
index 2a86ad1..ec44f26 100644
--- a/src/dawn/native/Buffer.cpp
+++ b/src/dawn/native/Buffer.cpp
@@ -590,40 +590,47 @@
// (note, not raise a validation error to the device) and return the null future.
DAWN_ASSERT(callbackInfo.nextInChain == nullptr);
- // Handle the defaulting of size required by WebGPU, even if in webgpu_cpp.h it is not
- // possible to default the function argument (because there is the callback later in the
- // argument list)
- if ((size == wgpu::kWholeMapSize) && (offset <= mSize)) {
- size = mSize - offset;
- }
-
- auto earlyStatus = [&]() -> std::optional<wgpu::BufferMapAsyncStatus> {
- if (mState == BufferState::PendingMap) {
- return wgpu::BufferMapAsyncStatus::MappingAlreadyPending;
- }
- WGPUBufferMapAsyncStatus status;
- if (GetDevice()->ConsumedError(ValidateMapAsync(mode, offset, size, &status),
- "calling %s.MapAsync(%s, %u, %u, ...).", this, mode, offset,
- size)) {
- return static_cast<wgpu::BufferMapAsyncStatus>(status);
- }
- if (GetDevice()->ConsumedError(MapAsyncImpl(mode, offset, size))) {
- return wgpu::BufferMapAsyncStatus::DeviceLost;
- }
- return std::nullopt;
- }();
-
Ref<EventManager::TrackedEvent> event;
- if (earlyStatus) {
- event = AcquireRef(new MapAsyncEvent(GetDevice(), callbackInfo, *earlyStatus));
- } else {
- mMapMode = mode;
- mMapOffset = offset;
- mMapSize = size;
- mState = BufferState::PendingMap;
- mPendingMapEvent =
- AcquireRef(new MapAsyncEvent(GetDevice(), this, callbackInfo, mLastUsageSerial));
- event = mPendingMapEvent;
+ std::optional<wgpu::BufferMapAsyncStatus> earlyStatus;
+ {
+ // TODO(crbug.com/dawn/831) Manually acquire device lock instead of relying on code-gen for
+ // re-entrancy.
+ auto deviceLock(GetDevice()->GetScopedLock());
+
+ // Handle the defaulting of size required by WebGPU, even if in webgpu_cpp.h it is not
+ // possible to default the function argument (because there is the callback later in the
+ // argument list)
+ if ((size == wgpu::kWholeMapSize) && (offset <= mSize)) {
+ size = mSize - offset;
+ }
+
+ earlyStatus = [&]() -> std::optional<wgpu::BufferMapAsyncStatus> {
+ if (mState == BufferState::PendingMap) {
+ return wgpu::BufferMapAsyncStatus::MappingAlreadyPending;
+ }
+ WGPUBufferMapAsyncStatus status;
+ if (GetDevice()->ConsumedError(ValidateMapAsync(mode, offset, size, &status),
+ "calling %s.MapAsync(%s, %u, %u, ...).", this, mode,
+ offset, size)) {
+ return static_cast<wgpu::BufferMapAsyncStatus>(status);
+ }
+ if (GetDevice()->ConsumedError(MapAsyncImpl(mode, offset, size))) {
+ return wgpu::BufferMapAsyncStatus::DeviceLost;
+ }
+ return std::nullopt;
+ }();
+
+ if (earlyStatus) {
+ event = AcquireRef(new MapAsyncEvent(GetDevice(), callbackInfo, *earlyStatus));
+ } else {
+ mMapMode = mode;
+ mMapOffset = offset;
+ mMapSize = size;
+ mState = BufferState::PendingMap;
+ mPendingMapEvent =
+ AcquireRef(new MapAsyncEvent(GetDevice(), this, callbackInfo, mLastUsageSerial));
+ event = mPendingMapEvent;
+ }
}
FutureID futureID = GetInstance()->GetEventManager()->TrackEvent(std::move(event));
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index 93fa3b6..9606b59 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -771,10 +771,16 @@
};
std::optional<ErrorScope> scope;
- if (IsLost()) {
- scope = ErrorScope(wgpu::ErrorType::DeviceLost, "GPU device disconnected");
- } else if (!mErrorScopeStack->Empty()) {
- scope = mErrorScopeStack->Pop();
+ {
+ // TODO(crbug.com/dawn/831) Manually acquire device lock instead of relying on code-gen for
+ // re-entrancy.
+ auto deviceLock(GetScopedLock());
+
+ if (IsLost()) {
+ scope = ErrorScope(wgpu::ErrorType::DeviceLost, "GPU device disconnected");
+ } else if (!mErrorScopeStack->Empty()) {
+ scope = mErrorScopeStack->Pop();
+ }
}
FutureID futureID = GetInstance()->GetEventManager()->TrackEvent(
diff --git a/src/dawn/native/Queue.cpp b/src/dawn/native/Queue.cpp
index 98c046d..6610a3c 100644
--- a/src/dawn/native/Queue.cpp
+++ b/src/dawn/native/Queue.cpp
@@ -322,20 +322,25 @@
DAWN_ASSERT(callbackInfo.nextInChain == nullptr);
Ref<EventManager::TrackedEvent> event;
+ {
+ // TODO(crbug.com/dawn/831) Manually acquire device lock instead of relying on code-gen for
+ // re-entrancy.
+ auto deviceLock(GetDevice()->GetScopedLock());
- wgpu::QueueWorkDoneStatus validationEarlyStatus;
- if (GetDevice()->ConsumedError(ValidateOnSubmittedWorkDone(&validationEarlyStatus))) {
- // TODO(crbug.com/dawn/2021): This is here to pretend that things succeed when the device is
- // lost. When the old OnSubmittedWorkDone is removed then we can update
- // ValidateOnSubmittedWorkDone to just return the correct thing here.
- if (validationEarlyStatus == wgpu::QueueWorkDoneStatus::DeviceLost) {
- validationEarlyStatus = wgpu::QueueWorkDoneStatus::Success;
+ wgpu::QueueWorkDoneStatus validationEarlyStatus;
+ if (GetDevice()->ConsumedError(ValidateOnSubmittedWorkDone(&validationEarlyStatus))) {
+ // TODO(crbug.com/dawn/2021): This is here to pretend that things succeed when the
+ // device is lost. When the old OnSubmittedWorkDone is removed then we can update
+ // ValidateOnSubmittedWorkDone to just return the correct thing here.
+ if (validationEarlyStatus == wgpu::QueueWorkDoneStatus::DeviceLost) {
+ validationEarlyStatus = wgpu::QueueWorkDoneStatus::Success;
+ }
+
+ // Note: if the callback is spontaneous, it'll get called in here.
+ event = AcquireRef(new WorkDoneEvent(callbackInfo, this, validationEarlyStatus));
+ } else {
+ event = AcquireRef(new WorkDoneEvent(callbackInfo, this, GetScheduledWorkDoneSerial()));
}
-
- // Note: if the callback is spontaneous, it'll get called in here.
- event = AcquireRef(new WorkDoneEvent(callbackInfo, this, validationEarlyStatus));
- } else {
- event = AcquireRef(new WorkDoneEvent(callbackInfo, this, GetScheduledWorkDoneSerial()));
}
FutureID futureID = GetInstance()->GetEventManager()->TrackEvent(std::move(event));
diff --git a/src/dawn/tests/end2end/BufferTests.cpp b/src/dawn/tests/end2end/BufferTests.cpp
index d057ec6..f7da0de 100644
--- a/src/dawn/tests/end2end/BufferTests.cpp
+++ b/src/dawn/tests/end2end/BufferTests.cpp
@@ -52,12 +52,6 @@
DAWN_TEST_UNSUPPORTED_IF(UsesWire() && GetParam().mFutureCallbackMode &&
*GetParam().mFutureCallbackMode ==
wgpu::CallbackMode::WaitAnyOnly);
-
- // TODO(dawn:2449): These tests are hitting an ASSERT in Mutex.cpp on Windows with the
- // spontaneous callback mode.
- DAWN_SUPPRESS_TEST_IF(IsWindows() && GetParam().mFutureCallbackMode &&
- GetParam().mFutureCallbackMode.value() ==
- wgpu::CallbackMode::AllowSpontaneous);
}
void MapAsyncAndWait(const wgpu::Buffer& buffer,
