From b2b08122ea72ef4bb42e0dc99c0e50b7f4cf2f91 Mon Sep 17 00:00:00 2001 From: Kevin Eady <8634912+KevinEady@users.noreply.github.com> Date: Fri, 7 Jun 2019 18:06:46 +0200 Subject: [PATCH] AsyncWorker: make callback optional `AsyncWorker` assumed that after work is complete, a JavaScript callback would need to execute. This change removes the restriction of specifying a `Function` callback, and instead replaces it with an `Env` parameter. Since the purpose of `receiver` was for the `this` context for the callback, it has also been removed from the constructors. Re: https://github.com/nodejs/node-addon-api/issues/231#issuecomment-483356091 PR-URL: https://github.com/nodejs/node-addon-api/pull/489 Reviewed-By: Michael Dawson --- doc/async_worker.md | 60 ++++++++++++++++++++++++++++++---- napi-inl.h | 34 +++++++++++++++++-- napi.h | 7 ++++ test/asyncworker-nocallback.js | 15 +++++++++ test/asyncworker.cc | 33 +++++++++++++++++++ test/index.js | 1 + 6 files changed, 142 insertions(+), 8 deletions(-) create mode 100644 test/asyncworker-nocallback.js diff --git a/doc/async_worker.md b/doc/async_worker.md index da12cba70..0516af8f6 100644 --- a/doc/async_worker.md +++ b/doc/async_worker.md @@ -105,8 +105,8 @@ virtual void Napi::AsyncWorker::Execute() = 0; ### OnOK -This method is invoked when the computation in the `Excecute` method ends. -The default implementation runs the Callback provided when the AsyncWorker class +This method is invoked when the computation in the `Execute` method ends. +The default implementation runs the Callback optionally provided when the AsyncWorker class was created. ```cpp @@ -149,7 +149,7 @@ explicit Napi::AsyncWorker(const Napi::Function& callback); - `[in] callback`: The function which will be called when an asynchronous operations ends. The given function is called from the main event loop thread. -Returns a`Napi::AsyncWork` instance which can later be queued for execution by calling +Returns a `Napi::AsyncWorker` instance which can later be queued for execution by calling `Queue`. ### Constructor @@ -166,7 +166,7 @@ operations ends. The given function is called from the main event loop thread. identifier for the kind of resource that is being provided for diagnostic information exposed by the async_hooks API. -Returns a `Napi::AsyncWork` instance which can later be queued for execution by +Returns a `Napi::AsyncWorker` instance which can later be queued for execution by calling `Napi::AsyncWork::Queue`. ### Constructor @@ -185,7 +185,7 @@ information exposed by the async_hooks API. - `[in] resource`: Object associated with the asynchronous operation that will be passed to possible async_hooks. -Returns a `Napi::AsyncWork` instance which can later be queued for execution by +Returns a `Napi::AsyncWorker` instance which can later be queued for execution by calling `Napi::AsyncWork::Queue`. ### Constructor @@ -200,7 +200,7 @@ explicit Napi::AsyncWorker(const Napi::Object& receiver, const Napi::Function& c - `[in] callback`: The function which will be called when an asynchronous operations ends. The given function is called from the main event loop thread. -Returns a `Napi::AsyncWork` instance which can later be queued for execution by +Returns a `Napi::AsyncWorker` instance which can later be queued for execution by calling `Napi::AsyncWork::Queue`. ### Constructor @@ -241,6 +241,54 @@ will be passed to possible async_hooks. Returns a `Napi::AsyncWork` instance which can later be queued for execution by calling `Napi::AsyncWork::Queue`. + +### Constructor + +Creates a new `Napi::AsyncWorker`. + +```cpp +explicit Napi::AsyncWorker(Napi::Env env); +``` + +- `[in] env`: The environment in which to create the `Napi::AsyncWorker`. + +Returns an `Napi::AsyncWorker` instance which can later be queued for execution by calling +`Napi::AsyncWorker::Queue`. + +### Constructor + +Creates a new `Napi::AsyncWorker`. + +```cpp +explicit Napi::AsyncWorker(Napi::Env env, const char* resource_name); +``` + +- `[in] env`: The environment in which to create the `Napi::AsyncWorker`. +- `[in] resource_name`: Null-terminated strings that represents the +identifier for the kind of resource that is being provided for diagnostic +information exposed by the async_hooks API. + +Returns a `Napi::AsyncWorker` instance which can later be queued for execution by +calling `Napi::AsyncWorker::Queue`. + +### Constructor + +Creates a new `Napi::AsyncWorker`. + +```cpp +explicit Napi::AsyncWorker(Napi::Env env, const char* resource_name, const Napi::Object& resource); +``` + +- `[in] env`: The environment in which to create the `Napi::AsyncWorker`. +- `[in] resource_name`: Null-terminated strings that represents the +identifier for the kind of resource that is being provided for diagnostic +information exposed by the async_hooks API. +- `[in] resource`: Object associated with the asynchronous operation that +will be passed to possible async_hooks. + +Returns a `Napi::AsyncWorker` instance which can later be queued for execution by +calling `Napi::AsyncWorker::Queue`. + ### Destructor Deletes the created work object that is used to execute logic asynchronously. diff --git a/napi-inl.h b/napi-inl.h index 9f03f3acc..7f94d4858 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -3522,6 +3522,32 @@ inline AsyncWorker::AsyncWorker(const Object& receiver, NAPI_THROW_IF_FAILED_VOID(_env, status); } +inline AsyncWorker::AsyncWorker(Napi::Env env) + : AsyncWorker(env, "generic") { +} + +inline AsyncWorker::AsyncWorker(Napi::Env env, + const char* resource_name) + : AsyncWorker(env, resource_name, Object::New(env)) { +} + +inline AsyncWorker::AsyncWorker(Napi::Env env, + const char* resource_name, + const Object& resource) + : _env(env), + _receiver(), + _callback(), + _suppress_destruct(false) { + napi_value resource_id; + napi_status status = napi_create_string_latin1( + _env, resource_name, NAPI_AUTO_LENGTH, &resource_id); + NAPI_THROW_IF_FAILED_VOID(_env, status); + + status = napi_create_async_work(_env, resource, resource_id, OnExecute, + OnWorkComplete, this, &_work); + NAPI_THROW_IF_FAILED_VOID(_env, status); +} + inline AsyncWorker::~AsyncWorker() { if (_work != nullptr) { napi_delete_async_work(_env, _work); @@ -3587,11 +3613,15 @@ inline void AsyncWorker::SuppressDestruct() { } inline void AsyncWorker::OnOK() { - _callback.Call(_receiver.Value(), std::initializer_list{}); + if (!_callback.IsEmpty()) { + _callback.Call(_receiver.Value(), std::initializer_list{}); + } } inline void AsyncWorker::OnError(const Error& e) { - _callback.Call(_receiver.Value(), std::initializer_list{ e.Value() }); + if (!_callback.IsEmpty()) { + _callback.Call(_receiver.Value(), std::initializer_list{ e.Value() }); + } } inline void AsyncWorker::SetError(const std::string& error) { diff --git a/napi.h b/napi.h index bf565fb91..799576112 100644 --- a/napi.h +++ b/napi.h @@ -1800,6 +1800,13 @@ namespace Napi { const char* resource_name, const Object& resource); + explicit AsyncWorker(Napi::Env env); + explicit AsyncWorker(Napi::Env env, + const char* resource_name); + explicit AsyncWorker(Napi::Env env, + const char* resource_name, + const Object& resource); + virtual void Execute() = 0; virtual void OnOK(); virtual void OnError(const Error& e); diff --git a/test/asyncworker-nocallback.js b/test/asyncworker-nocallback.js new file mode 100644 index 000000000..c873c5fbf --- /dev/null +++ b/test/asyncworker-nocallback.js @@ -0,0 +1,15 @@ +'use strict'; +const buildType = process.config.target_defaults.default_configuration; +const common = require('./common'); + +test(require(`./build/${buildType}/binding.node`)); +test(require(`./build/${buildType}/binding_noexcept.node`)); + +function test(binding) { + const resolving = binding.asyncworker.doWorkNoCallback(true, {}); + resolving.then(common.mustCall()).catch(common.mustNotCall()); + + const rejecting = binding.asyncworker.doWorkNoCallback(false, {}); + rejecting.then(common.mustNotCall()).catch(common.mustCall()); + return; +} \ No newline at end of file diff --git a/test/asyncworker.cc b/test/asyncworker.cc index 12cdcfabe..bbd7e0b19 100644 --- a/test/asyncworker.cc +++ b/test/asyncworker.cc @@ -29,8 +29,41 @@ class TestWorker : public AsyncWorker { bool _succeed; }; +class TestWorkerNoCallback : public AsyncWorker { +public: + static Value DoWork(const CallbackInfo& info) { + napi_env env = info.Env(); + bool succeed = info[0].As(); + Object resource = info[1].As(); + + TestWorkerNoCallback* worker = new TestWorkerNoCallback(env, "TestResource", resource); + worker->_succeed = succeed; + worker->Queue(); + return worker->_deferred.Promise(); + } + +protected: + void Execute() override { + } + virtual void OnOK() override { + _deferred.Resolve(Env().Undefined()); + + } + virtual void OnError(const Napi::Error& /* e */) override { + _deferred.Reject(Env().Undefined()); + } + +private: + TestWorkerNoCallback(napi_env env, const char* resource_name, const Object& resource) + : AsyncWorker(env, resource_name, resource), _deferred(Napi::Promise::Deferred::New(env)) { + } + Promise::Deferred _deferred; + bool _succeed; +}; + Object InitAsyncWorker(Env env) { Object exports = Object::New(env); exports["doWork"] = Function::New(env, TestWorker::DoWork); + exports["doWorkNoCallback"] = Function::New(env, TestWorkerNoCallback::DoWork); return exports; } diff --git a/test/index.js b/test/index.js index 67e630d7f..e46c7b162 100644 --- a/test/index.js +++ b/test/index.js @@ -11,6 +11,7 @@ let testModules = [ 'arraybuffer', 'asynccontext', 'asyncworker', + 'asyncworker-nocallback', 'asyncworker-persistent', 'basic_types/array', 'basic_types/boolean',