Skip to content

Commit

Permalink
src: make OnWorkComplete and OnExecute override-able
Browse files Browse the repository at this point in the history 
No breaking changes on existing code were expected. All existing tests
shall pass without any touch.

Changes on declaration:
- Added `Napi::AsyncWorker::OnWorkComplete`.
- Added `Napi::AsyncWorker::OnExecute`.

PR-URL: #589
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
@legendecas
legendecas authored and Gabriel Schulhof committed Jan 10, 2020
1 parent 86384f9 commit 23ff7f0
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 54 deletions.
29 changes: 29 additions & 0 deletions doc/async_worker.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,35 @@ class was created, passing in the error as the first parameter.
virtual void Napi::AsyncWorker::OnError(const Napi::Error& e);
```

### OnWorkComplete

This method is invoked after the work has completed on JavaScript thread.
The default implementation of this method checks the status of the work and
tries to dispatch the result to `Napi::AsyncWorker::OnOk` or `Napi::AsyncWorker::Error`
if the work has committed an error. If the work was cancelled, neither
`Napi::AsyncWorker::OnOk` nor `Napi::AsyncWorker::Error` will be invoked.
After the result is dispatched, the default implementation will call into
`Napi::AsyncWorker::Destroy` if `SuppressDestruct()` was not called.

```cpp
virtual void OnWorkComplete(Napi::Env env, napi_status status);
```
### OnExecute
This method is invoked immediately on the work thread when scheduled.
The default implementation of this method just calls the `Napi::AsyncWorker::Execute`
and handles exceptions if cpp exceptions were enabled.
The `OnExecute` method receives an `napi_env` argument. However, the `napi_env`
must NOT be used within this method, as it does not run on the JavaScript
thread and must not run any method that would cause JavaScript to run. In
practice, this means that almost any use of `napi_env` will be incorrect.
```cpp
virtual void OnExecute(Napi::Env env);
```

### Destroy

This method is invoked when the instance must be deallocated. If
Expand Down
66 changes: 38 additions & 28 deletions napi-inl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4120,8 +4120,8 @@ inline AsyncWorker::AsyncWorker(const Object& receiver,
_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);
status = napi_create_async_work(_env, resource, resource_id, OnAsyncWorkExecute,
OnAsyncWorkComplete, this, &_work);
NAPI_THROW_IF_FAILED_VOID(_env, status);
}

Expand All @@ -4146,8 +4146,8 @@ inline AsyncWorker::AsyncWorker(Napi::Env env,
_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);
status = napi_create_async_work(_env, resource, resource_id, OnAsyncWorkExecute,
OnAsyncWorkComplete, this, &_work);
NAPI_THROW_IF_FAILED_VOID(_env, status);
}

Expand Down Expand Up @@ -4234,40 +4234,51 @@ inline void AsyncWorker::SetError(const std::string& error) {
inline std::vector<napi_value> AsyncWorker::GetResult(Napi::Env /*env*/) {
return {};
}
// The OnAsyncWorkExecute method receives an napi_env argument. However, do NOT
// use it within this method, as it does not run on the JavaScript thread and
// must not run any method that would cause JavaScript to run. In practice,
// this means that almost any use of napi_env will be incorrect.
inline void AsyncWorker::OnAsyncWorkExecute(napi_env env, void* asyncworker) {
AsyncWorker* self = static_cast<AsyncWorker*>(asyncworker);
self->OnExecute(env);
}
// The OnExecute method receives an napi_env argument. However, do NOT
// use it within this method, as it does not run on the main thread and must
// not run any method that would cause JavaScript to run. In practice, this
// means that almost any use of napi_env will be incorrect.
inline void AsyncWorker::OnExecute(napi_env /*DO_NOT_USE*/, void* this_pointer) {
AsyncWorker* self = static_cast<AsyncWorker*>(this_pointer);
// use it within this method, as it does not run on the JavaScript thread and
// must not run any method that would cause JavaScript to run. In practice,
// this means that almost any use of napi_env will be incorrect.
inline void AsyncWorker::OnExecute(Napi::Env /*DO_NOT_USE*/) {
#ifdef NAPI_CPP_EXCEPTIONS
try {
self->Execute();
Execute();
} catch (const std::exception& e) {
self->SetError(e.what());
SetError(e.what());
}
#else // NAPI_CPP_EXCEPTIONS
self->Execute();
Execute();
#endif // NAPI_CPP_EXCEPTIONS
}

inline void AsyncWorker::OnWorkComplete(
napi_env /*env*/, napi_status status, void* this_pointer) {
AsyncWorker* self = static_cast<AsyncWorker*>(this_pointer);
inline void AsyncWorker::OnAsyncWorkComplete(napi_env env,
napi_status status,
void* asyncworker) {
AsyncWorker* self = static_cast<AsyncWorker*>(asyncworker);
self->OnWorkComplete(env, status);
}
inline void AsyncWorker::OnWorkComplete(Napi::Env /*env*/, napi_status status) {
if (status != napi_cancelled) {
HandleScope scope(self->_env);
HandleScope scope(_env);
details::WrapCallback([&] {
if (self->_error.size() == 0) {
self->OnOK();
if (_error.size() == 0) {
OnOK();
}
else {
self->OnError(Error::New(self->_env, self->_error));
OnError(Error::New(_env, _error));
}
return nullptr;
});
}
if (!self->_suppress_destruct) {
self->Destroy();
if (!_suppress_destruct) {
Destroy();
}
}

Expand Down Expand Up @@ -4632,14 +4643,14 @@ inline AsyncProgressWorker<T>::AsyncProgressWorker(const Function& callback,

template<class T>
inline AsyncProgressWorker<T>::AsyncProgressWorker(const Object& receiver,
const Function& callback)
const Function& callback)
: AsyncProgressWorker(receiver, callback, "generic") {
}

template<class T>
inline AsyncProgressWorker<T>::AsyncProgressWorker(const Object& receiver,
const Function& callback,
const char* resource_name)
const Function& callback,
const char* resource_name)
: AsyncProgressWorker(receiver,
callback,
resource_name,
Expand All @@ -4665,14 +4676,14 @@ inline AsyncProgressWorker<T>::AsyncProgressWorker(Napi::Env env)

template<class T>
inline AsyncProgressWorker<T>::AsyncProgressWorker(Napi::Env env,
const char* resource_name)
const char* resource_name)
: AsyncProgressWorker(env, resource_name, Object::New(env)) {
}

template<class T>
inline AsyncProgressWorker<T>::AsyncProgressWorker(Napi::Env env,
const char* resource_name,
const Object& resource)
const char* resource_name,
const Object& resource)
: AsyncWorker(env, resource_name, resource),
_asyncdata(nullptr),
_asyncsize(0) {
Expand Down Expand Up @@ -4751,7 +4762,6 @@ template<class T>
inline void AsyncProgressWorker<T>::ExecutionProgress::Send(const T* data, size_t count) const {
_worker->SendProgress_(data, count);
}

#endif

////////////////////////////////////////////////////////////////////////////////
Expand Down
55 changes: 29 additions & 26 deletions napi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1986,6 +1986,10 @@ namespace Napi {
ObjectReference& Receiver();
FunctionReference& Callback();

virtual void OnExecute(Napi::Env env);
virtual void OnWorkComplete(Napi::Env env,
napi_status status);

protected:
explicit AsyncWorker(const Function& callback);
explicit AsyncWorker(const Function& callback,
Expand Down Expand Up @@ -2019,10 +2023,10 @@ namespace Napi {
void SetError(const std::string& error);

private:
static void OnExecute(napi_env env, void* this_pointer);
static void OnWorkComplete(napi_env env,
napi_status status,
void* this_pointer);
static inline void OnAsyncWorkExecute(napi_env env, void* asyncworker);
static inline void OnAsyncWorkComplete(napi_env env,
napi_status status,
void* asyncworker);

napi_env _env;
napi_async_work _work;
Expand Down Expand Up @@ -2254,33 +2258,32 @@ namespace Napi {
};

protected:
explicit AsyncProgressWorker(const Function& callback);
explicit AsyncProgressWorker(const Function& callback,
const char* resource_name);
explicit AsyncProgressWorker(const Function& callback,
const char* resource_name,
const Object& resource);
explicit AsyncProgressWorker(const Object& receiver,
const Function& callback);
explicit AsyncProgressWorker(const Object& receiver,
const Function& callback,
const char* resource_name);
explicit AsyncProgressWorker(const Object& receiver,
const Function& callback,
const char* resource_name,
const Object& resource);
explicit AsyncProgressWorker(const Function& callback);
explicit AsyncProgressWorker(const Function& callback,
const char* resource_name);
explicit AsyncProgressWorker(const Function& callback,
const char* resource_name,
const Object& resource);
explicit AsyncProgressWorker(const Object& receiver,
const Function& callback);
explicit AsyncProgressWorker(const Object& receiver,
const Function& callback,
const char* resource_name);
explicit AsyncProgressWorker(const Object& receiver,
const Function& callback,
const char* resource_name,
const Object& resource);

// Optional callback of Napi::ThreadSafeFunction only available after NAPI_VERSION 4.
// Refs: https://github.com/nodejs/node/pull/27791
#if NAPI_VERSION > 4
explicit AsyncProgressWorker(Napi::Env env);
explicit AsyncProgressWorker(Napi::Env env,
const char* resource_name);
explicit AsyncProgressWorker(Napi::Env env,
const char* resource_name,
const Object& resource);
explicit AsyncProgressWorker(Napi::Env env);
explicit AsyncProgressWorker(Napi::Env env,
const char* resource_name);
explicit AsyncProgressWorker(Napi::Env env,
const char* resource_name,
const Object& resource);
#endif

virtual void Execute(const ExecutionProgress& progress) = 0;
virtual void OnProgress(const T* data, size_t count) = 0;

Expand Down

0 comments on commit 23ff7f0

Please sign in to comment.