dawn_node: Track promises
These should always be resolved or rejected.
The Fatal() call, when a promise is not resolved or rejected, is currently disabled due to https://github.com/gpuweb/cts/issues/784.
Bug: dawn:1123
Change-Id: Ie0e8ac187ad70be0fea41cd66956d0bfd9c53212
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/66821
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/dawn_node/binding/GPU.cpp b/src/dawn_node/binding/GPU.cpp
index 2f64978..61e4d6f 100644
--- a/src/dawn_node/binding/GPU.cpp
+++ b/src/dawn_node/binding/GPU.cpp
@@ -55,8 +55,8 @@
interop::Promise<std::optional<interop::Interface<interop::GPUAdapter>>> GPU::requestAdapter(
Napi::Env env,
interop::GPURequestAdapterOptions options) {
- auto promise =
- interop::Promise<std::optional<interop::Interface<interop::GPUAdapter>>>(env);
+ auto promise = interop::Promise<std::optional<interop::Interface<interop::GPUAdapter>>>(
+ env, PROMISE_INFO);
if (options.forceFallbackAdapter) {
// Software adapters are not currently supported.
diff --git a/src/dawn_node/binding/GPUAdapter.cpp b/src/dawn_node/binding/GPUAdapter.cpp
index 82378d8..c49e442 100644
--- a/src/dawn_node/binding/GPUAdapter.cpp
+++ b/src/dawn_node/binding/GPUAdapter.cpp
@@ -139,7 +139,7 @@
Napi::Env env,
interop::GPUDeviceDescriptor descriptor) {
dawn_native::DeviceDescriptor desc{}; // TODO(crbug.com/dawn/1133): Fill in.
- interop::Promise<interop::Interface<interop::GPUDevice>> promise(env);
+ interop::Promise<interop::Interface<interop::GPUDevice>> promise(env, PROMISE_INFO);
// See src/dawn_native/Features.cpp for enum <-> string mappings.
for (auto required : descriptor.requiredFeatures) {
diff --git a/src/dawn_node/binding/GPUBuffer.cpp b/src/dawn_node/binding/GPUBuffer.cpp
index ac5ae54..2fca5d5 100644
--- a/src/dawn_node/binding/GPUBuffer.cpp
+++ b/src/dawn_node/binding/GPUBuffer.cpp
@@ -47,11 +47,13 @@
wgpu::MapMode md{};
Converter conv(env);
if (!conv(md, mode)) {
- return {env};
+ interop::Promise<void> promise(env, PROMISE_INFO);
+ promise.Reject(Errors::OperationError(env));
+ return promise;
}
if (state_ != State::Unmapped) {
- interop::Promise<void> promise(env);
+ interop::Promise<void> promise(env, PROMISE_INFO);
promise.Reject(Errors::OperationError(env));
device_.InjectError(wgpu::ErrorType::Validation,
"mapAsync called on buffer that is not in the unmapped state");
@@ -64,7 +66,7 @@
AsyncTask task;
State& state;
};
- auto ctx = new Context{env, interop::Promise<void>(env), async_, state_};
+ auto ctx = new Context{env, interop::Promise<void>(env, PROMISE_INFO), async_, state_};
auto promise = ctx->promise;
uint64_t s = size.has_value() ? size.value() : (desc_.size - offset);
diff --git a/src/dawn_node/binding/GPUDevice.cpp b/src/dawn_node/binding/GPUDevice.cpp
index 0870307..2f928e5 100644
--- a/src/dawn_node/binding/GPUDevice.cpp
+++ b/src/dawn_node/binding/GPUDevice.cpp
@@ -138,7 +138,12 @@
return interop::GPUQueue::Create<GPUQueue>(env, device_.GetQueue(), async_);
}
- void GPUDevice::destroy(Napi::Env) {
+ void GPUDevice::destroy(Napi::Env env) {
+ for (auto promise : lost_promises_) {
+ promise.Resolve(interop::GPUDeviceLostInfo::Create<DeviceLostInfo>(
+ env_, interop::GPUDeviceLostReason::kDestroyed, "device was destroyed"));
+ }
+ lost_promises_.clear();
device_.Release();
}
@@ -293,21 +298,23 @@
interop::Promise<interop::Interface<interop::GPUComputePipeline>>
GPUDevice::createComputePipelineAsync(Napi::Env env,
interop::GPUComputePipelineDescriptor descriptor) {
+ using Promise = interop::Promise<interop::Interface<interop::GPUComputePipeline>>;
+
Converter conv(env);
wgpu::ComputePipelineDescriptor desc{};
if (!conv(desc, descriptor)) {
- return {env};
+ Promise promise(env, PROMISE_INFO);
+ promise.Reject(Errors::OperationError(env));
+ return promise;
}
- using Promise = interop::Promise<interop::Interface<interop::GPUComputePipeline>>;
-
struct Context {
Napi::Env env;
Promise promise;
AsyncTask task;
};
- auto ctx = new Context{env, env, async_};
+ auto ctx = new Context{env, Promise(env, PROMISE_INFO), async_};
auto promise = ctx->promise;
device_.CreateComputePipelineAsync(
@@ -334,21 +341,23 @@
interop::Promise<interop::Interface<interop::GPURenderPipeline>>
GPUDevice::createRenderPipelineAsync(Napi::Env env,
interop::GPURenderPipelineDescriptor descriptor) {
+ using Promise = interop::Promise<interop::Interface<interop::GPURenderPipeline>>;
+
Converter conv(env);
wgpu::RenderPipelineDescriptor desc{};
if (!conv(desc, descriptor)) {
- return {env};
+ Promise promise(env, PROMISE_INFO);
+ promise.Reject(Errors::OperationError(env));
+ return promise;
}
- using Promise = interop::Promise<interop::Interface<interop::GPURenderPipeline>>;
-
struct Context {
Napi::Env env;
Promise promise;
AsyncTask task;
};
- auto ctx = new Context{env, env, async_};
+ auto ctx = new Context{env, Promise(env, PROMISE_INFO), async_};
auto promise = ctx->promise;
device_.CreateRenderPipelineAsync(
@@ -415,7 +424,8 @@
interop::Promise<interop::Interface<interop::GPUDeviceLostInfo>> GPUDevice::getLost(
Napi::Env env) {
- auto promise = interop::Promise<interop::Interface<interop::GPUDeviceLostInfo>>(env);
+ auto promise =
+ interop::Promise<interop::Interface<interop::GPUDeviceLostInfo>>(env, PROMISE_INFO);
lost_promises_.emplace_back(promise);
return promise;
}
@@ -444,7 +454,7 @@
Promise promise;
AsyncTask task;
};
- auto* ctx = new Context{env, env, async_};
+ auto* ctx = new Context{env, Promise(env, PROMISE_INFO), async_};
auto promise = ctx->promise;
bool ok = device_.PopErrorScope(
@@ -476,10 +486,8 @@
}
delete ctx;
- Promise p(env);
- p.Resolve(
- interop::GPUValidationError::Create<ValidationError>(env, "failed to pop error scope"));
- return p;
+ promise.Reject(Errors::OperationError(env));
+ return promise;
}
std::optional<std::string> GPUDevice::getLabel(Napi::Env) {
diff --git a/src/dawn_node/binding/GPUQueue.cpp b/src/dawn_node/binding/GPUQueue.cpp
index c8e39fe..e1c0413 100644
--- a/src/dawn_node/binding/GPUQueue.cpp
+++ b/src/dawn_node/binding/GPUQueue.cpp
@@ -51,7 +51,7 @@
interop::Promise<void> promise;
AsyncTask task;
};
- auto ctx = new Context{env, interop::Promise<void>(env), async_};
+ auto ctx = new Context{env, interop::Promise<void>(env, PROMISE_INFO), async_};
auto promise = ctx->promise;
queue_.OnSubmittedWorkDone(
diff --git a/src/dawn_node/binding/GPUShaderModule.cpp b/src/dawn_node/binding/GPUShaderModule.cpp
index 3323ba5..52efabd 100644
--- a/src/dawn_node/binding/GPUShaderModule.cpp
+++ b/src/dawn_node/binding/GPUShaderModule.cpp
@@ -91,7 +91,7 @@
Promise promise;
AsyncTask task;
};
- auto ctx = new Context{env, env, async_};
+ auto ctx = new Context{env, Promise(env, PROMISE_INFO), async_};
auto promise = ctx->promise;
shader_.GetCompilationInfo(
diff --git a/src/dawn_node/interop/Core.h b/src/dawn_node/interop/Core.h
index ff26930..dba1ad0 100644
--- a/src/dawn_node/interop/Core.h
+++ b/src/dawn_node/interop/Core.h
@@ -38,6 +38,13 @@
# define INTEROP_LOG(...)
#endif
+// A helper macro for constructing a PromiseInfo with the current file, function and line.
+// See PromiseInfo
+#define PROMISE_INFO \
+ ::wgpu::interop::PromiseInfo { \
+ __FILE__, __FUNCTION__, __LINE__ \
+ }
+
namespace wgpu { namespace interop {
////////////////////////////////////////////////////////////////////////////////
@@ -147,83 +154,106 @@
// Promise<T>
////////////////////////////////////////////////////////////////////////////////
+ // Info holds details about where the promise was constructed.
+ // Used for printing debug messages when a promise is finalized without being resolved
+ // or rejected.
+ // Use the PROMISE_INFO macro to populate this structure.
+ struct PromiseInfo {
+ const char* file = nullptr;
+ const char* function = nullptr;
+ int line = 0;
+ };
+
+ namespace detail {
+ // Base class for Promise<T> specializations.
+ class PromiseBase {
+ public:
+ // Implicit conversion operators to Napi promises.
+ inline operator napi_value() const {
+ return state->deferred.Promise();
+ }
+ inline operator Napi::Value() const {
+ return state->deferred.Promise();
+ }
+ inline operator Napi::Promise() const {
+ return state->deferred.Promise();
+ }
+
+ // Reject() rejects the promise with the given failure value.
+ void Reject(Napi::Value value) const {
+ state->deferred.Reject(value);
+ state->resolved_or_rejected = true;
+ }
+ void Reject(Napi::Error err) const {
+ Reject(err.Value());
+ }
+ void Reject(std::string err) const {
+ Reject(Napi::Error::New(state->deferred.Env(), err));
+ }
+
+ protected:
+ void Resolve(Napi::Value value) const {
+ state->deferred.Resolve(value);
+ state->resolved_or_rejected = true;
+ }
+
+ struct State {
+ Napi::Promise::Deferred deferred;
+ PromiseInfo info;
+ bool resolved_or_rejected = false;
+ };
+
+ PromiseBase(Napi::Env env, const PromiseInfo& info)
+ : state(new State{Napi::Promise::Deferred::New(env), info}) {
+ state->deferred.Promise().AddFinalizer(
+ [](Napi::Env, State* state) {
+ // TODO(https://github.com/gpuweb/cts/issues/784):
+ // Devices are never destroyed, so we always end up
+ // leaking the Device.lost promise. Enable this once
+ // fixed.
+ if ((false)) {
+ if (!state->resolved_or_rejected) {
+ ::wgpu::utils::Fatal("Promise not resolved or rejected",
+ state->info.file, state->info.line,
+ state->info.function);
+ }
+ }
+ delete state;
+ },
+ state);
+ }
+
+ State* const state;
+ };
+ } // namespace detail
+
// Promise<T> is a templated wrapper around a JavaScript promise, which can
// resolve to the template type T.
template <typename T>
- class Promise {
+ class Promise : public detail::PromiseBase {
public:
// Constructor
- Promise(Napi::Env env) : deferred(Napi::Promise::Deferred::New(env)) {
- }
-
- // Implicit conversion operators to Napi promises.
- inline operator napi_value() const {
- return deferred.Promise();
- }
- inline operator Napi::Value() const {
- return deferred.Promise();
- }
- inline operator Napi::Promise() const {
- return deferred.Promise();
+ Promise(Napi::Env env, const PromiseInfo& info) : PromiseBase(env, info) {
}
// Resolve() fulfills the promise with the given value.
void Resolve(T&& value) const {
- deferred.Resolve(ToJS(deferred.Env(), std::forward<T>(value)));
+ PromiseBase::Resolve(ToJS(state->deferred.Env(), std::forward<T>(value)));
}
-
- // Reject() rejects the promise with the given failure value.
- void Reject(Napi::Object obj) const {
- deferred.Reject(obj);
- }
- void Reject(Napi::Error err) const {
- deferred.Reject(err.Value());
- }
- void Reject(std::string err) const {
- Reject(Napi::Error::New(deferred.Env(), err));
- }
-
- private:
- Napi::Promise::Deferred deferred;
};
// Specialization for Promises that resolve with no value
template <>
- class Promise<void> {
+ class Promise<void> : public detail::PromiseBase {
public:
// Constructor
- Promise(Napi::Env env) : deferred(Napi::Promise::Deferred::New(env)) {
- }
-
- // Implicit conversion operators to Napi promises.
- inline operator napi_value() const {
- return deferred.Promise();
- }
- inline operator Napi::Value() const {
- return deferred.Promise();
- }
- inline operator Napi::Promise() const {
- return deferred.Promise();
+ Promise(Napi::Env env, const PromiseInfo& info) : PromiseBase(env, info) {
}
// Resolve() fulfills the promise.
void Resolve() const {
- deferred.Resolve(deferred.Env().Undefined());
+ PromiseBase::Resolve(state->deferred.Env().Undefined());
}
-
- // Reject() rejects the promise with the given failure value.
- void Reject(Napi::Object obj) const {
- deferred.Reject(obj);
- }
- void Reject(Napi::Error err) const {
- deferred.Reject(err.Value());
- }
- void Reject(std::string err) const {
- Reject(Napi::Error::New(deferred.Env(), err));
- }
-
- private:
- Napi::Promise::Deferred deferred;
};
////////////////////////////////////////////////////////////////////////////////
