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 gpuweb/cts#784. Bug: dawn:1123 Change-Id: Ie0e8ac187ad70be0fea41cd66956d0bfd9c53212 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/66821 Reviewed-by: Antonio Maiorano <[email protected]> Commit-Queue: Ben Clayton <[email protected]>
faro-oss · Oct 19, 2021 · e9cbd48 · e9cbd48
1 parent 0e3d4fc
commit e9cbd48
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 79 deletions.
diff --git a/src/dawn_node/binding/GPU.cpp b/src/dawn_node/binding/GPU.cpp
@@ -55,8 +55,8 @@ namespace wgpu { namespace binding {
     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
@@ -139,7 +139,7 @@ namespace wgpu { namespace binding {
         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
@@ -47,11 +47,13 @@ namespace wgpu { namespace binding {
         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 @@ namespace wgpu { namespace binding {
             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
@@ -138,7 +138,12 @@ namespace wgpu { namespace binding {
         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 @@ namespace wgpu { namespace binding {
     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 @@ namespace wgpu { namespace binding {
     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 @@ namespace wgpu { namespace binding {
 
     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 @@ namespace wgpu { namespace binding {
             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 @@ namespace wgpu { namespace binding {
         }
 
         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
@@ -51,7 +51,7 @@ namespace wgpu { namespace binding {
             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
@@ -91,7 +91,7 @@ namespace wgpu { namespace binding {
             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
@@ -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 @@ namespace wgpu { namespace interop {
     // 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)));
-        }
-
-        // Reject() rejects the promise with the given failure value.
-        void Reject(Napi::Object obj) const {
-            deferred.Reject(obj);
+            PromiseBase::Resolve(ToJS(state->deferred.Env(), std::forward<T>(value)));
         }
-        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());
-        }
-
-        // 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));
+            PromiseBase::Resolve(state->deferred.Env().Undefined());
         }
-
-      private:
-        Napi::Promise::Deferred deferred;
     };
 
     ////////////////////////////////////////////////////////////////////////////////