Skip to content

Commit

Permalink
AsyncWorker: make callback optional
Browse files Browse the repository at this point in the history
`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: nodejs/node-addon-api#231 (comment)

PR-URL: nodejs/node-addon-api#489
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
wroy7860 committed Jun 18, 2019
1 parent ab9f6f6 commit c21326e
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 8 deletions.
60 changes: 54 additions & 6 deletions doc/async_worker.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
34 changes: 32 additions & 2 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -3587,11 +3613,15 @@ inline void AsyncWorker::SuppressDestruct() {
}

inline void AsyncWorker::OnOK() {
_callback.Call(_receiver.Value(), std::initializer_list<napi_value>{});
if (!_callback.IsEmpty()) {
_callback.Call(_receiver.Value(), std::initializer_list<napi_value>{});
}
}

inline void AsyncWorker::OnError(const Error& e) {
_callback.Call(_receiver.Value(), std::initializer_list<napi_value>{ e.Value() });
if (!_callback.IsEmpty()) {
_callback.Call(_receiver.Value(), std::initializer_list<napi_value>{ e.Value() });
}
}

inline void AsyncWorker::SetError(const std::string& error) {
Expand Down
7 changes: 7 additions & 0 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
15 changes: 15 additions & 0 deletions test/asyncworker-nocallback.js
Original file line number Diff line number Diff line change
@@ -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;
}
33 changes: 33 additions & 0 deletions test/asyncworker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<Boolean>();
Object resource = info[1].As<Object>();

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;
}
1 change: 1 addition & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ let testModules = [
'arraybuffer',
'asynccontext',
'asyncworker',
'asyncworker-nocallback',
'asyncworker-persistent',
'basic_types/array',
'basic_types/boolean',
Expand Down

0 comments on commit c21326e

Please sign in to comment.