Skip to content

Commit 7683632

Browse files
kevindavies8kevindavies8
committed
src: make OnWorkComplete and OnExecute override-able
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: nodejs/node-addon-api#589 Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent 85435af commit 7683632

File tree

3 files changed

+96
-54
lines changed

3 files changed

+96
-54
lines changed

‎doc/async_worker.md‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,35 @@ class was created, passing in the error as the first parameter.
136136
virtual void Napi::AsyncWorker::OnError(const Napi::Error& e);
137137
```
138138

139+
### OnWorkComplete
140+
141+
This method is invoked after the work has completed on JavaScript thread.
142+
The default implementation of this method checks the status of the work and
143+
tries to dispatch the result to `Napi::AsyncWorker::OnOk` or `Napi::AsyncWorker::Error`
144+
if the work has committed an error. If the work was cancelled, neither
145+
`Napi::AsyncWorker::OnOk` nor `Napi::AsyncWorker::Error` will be invoked.
146+
After the result is dispatched, the default implementation will call into
147+
`Napi::AsyncWorker::Destroy` if `SuppressDestruct()` was not called.
148+
149+
```cpp
150+
virtual void OnWorkComplete(Napi::Env env, napi_status status);
151+
```
152+
153+
### OnExecute
154+
155+
This method is invoked immediately on the work thread when scheduled.
156+
The default implementation of this method just calls the `Napi::AsyncWorker::Execute`
157+
and handles exceptions if cpp exceptions were enabled.
158+
159+
The `OnExecute` method receives an `napi_env` argument. However, the `napi_env`
160+
must NOT be used within this method, as it does not run on the JavaScript
161+
thread and must not run any method that would cause JavaScript to run. In
162+
practice, this means that almost any use of `napi_env` will be incorrect.
163+
164+
```cpp
165+
virtual void OnExecute(Napi::Env env);
166+
```
167+
139168
### Destroy
140169

141170
This method is invoked when the instance must be deallocated. If

‎napi-inl.h‎

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4120,8 +4120,8 @@ inline AsyncWorker::AsyncWorker(const Object& receiver,
41204120
_env, resource_name, NAPI_AUTO_LENGTH, &resource_id);
41214121
NAPI_THROW_IF_FAILED_VOID(_env, status);
41224122

4123-
status = napi_create_async_work(_env, resource, resource_id, OnExecute,
4124-
OnWorkComplete, this, &_work);
4123+
status = napi_create_async_work(_env, resource, resource_id, OnAsyncWorkExecute,
4124+
OnAsyncWorkComplete, this, &_work);
41254125
NAPI_THROW_IF_FAILED_VOID(_env, status);
41264126
}
41274127

@@ -4146,8 +4146,8 @@ inline AsyncWorker::AsyncWorker(Napi::Env env,
41464146
_env, resource_name, NAPI_AUTO_LENGTH, &resource_id);
41474147
NAPI_THROW_IF_FAILED_VOID(_env, status);
41484148

4149-
status = napi_create_async_work(_env, resource, resource_id, OnExecute,
4150-
OnWorkComplete, this, &_work);
4149+
status = napi_create_async_work(_env, resource, resource_id, OnAsyncWorkExecute,
4150+
OnAsyncWorkComplete, this, &_work);
41514151
NAPI_THROW_IF_FAILED_VOID(_env, status);
41524152
}
41534153

@@ -4234,40 +4234,51 @@ inline void AsyncWorker::SetError(const std::string& error) {
42344234
inline std::vector<napi_value> AsyncWorker::GetResult(Napi::Env /*env*/) {
42354235
return {};
42364236
}
4237+
// The OnAsyncWorkExecute method receives an napi_env argument. However, do NOT
4238+
// use it within this method, as it does not run on the JavaScript thread and
4239+
// must not run any method that would cause JavaScript to run. In practice,
4240+
// this means that almost any use of napi_env will be incorrect.
4241+
inline void AsyncWorker::OnAsyncWorkExecute(napi_env env, void* asyncworker) {
4242+
AsyncWorker* self = static_cast<AsyncWorker*>(asyncworker);
4243+
self->OnExecute(env);
4244+
}
42374245
// The OnExecute method receives an napi_env argument. However, do NOT
4238-
// use it within this method, as it does not run on the main thread and must
4239-
// not run any method that would cause JavaScript to run. In practice, this
4240-
// means that almost any use of napi_env will be incorrect.
4241-
inline void AsyncWorker::OnExecute(napi_env /*DO_NOT_USE*/, void* this_pointer) {
4242-
AsyncWorker* self = static_cast<AsyncWorker*>(this_pointer);
4246+
// use it within this method, as it does not run on the JavaScript thread and
4247+
// must not run any method that would cause JavaScript to run. In practice,
4248+
// this means that almost any use of napi_env will be incorrect.
4249+
inline void AsyncWorker::OnExecute(Napi::Env /*DO_NOT_USE*/) {
42434250
#ifdef NAPI_CPP_EXCEPTIONS
42444251
try {
4245-
self->Execute();
4252+
Execute();
42464253
} catch (const std::exception& e) {
4247-
self->SetError(e.what());
4254+
SetError(e.what());
42484255
}
42494256
#else // NAPI_CPP_EXCEPTIONS
4250-
self->Execute();
4257+
Execute();
42514258
#endif // NAPI_CPP_EXCEPTIONS
42524259
}
42534260

4254-
inline void AsyncWorker::OnWorkComplete(
4255-
napi_env /*env*/, napi_status status, void* this_pointer) {
4256-
AsyncWorker* self = static_cast<AsyncWorker*>(this_pointer);
4261+
inline void AsyncWorker::OnAsyncWorkComplete(napi_env env,
4262+
napi_status status,
4263+
void* asyncworker) {
4264+
AsyncWorker* self = static_cast<AsyncWorker*>(asyncworker);
4265+
self->OnWorkComplete(env, status);
4266+
}
4267+
inline void AsyncWorker::OnWorkComplete(Napi::Env /*env*/, napi_status status) {
42574268
if (status != napi_cancelled) {
4258-
HandleScope scope(self->_env);
4269+
HandleScope scope(_env);
42594270
details::WrapCallback([&] {
4260-
if (self->_error.size() == 0) {
4261-
self->OnOK();
4271+
if (_error.size() == 0) {
4272+
OnOK();
42624273
}
42634274
else {
4264-
self->OnError(Error::New(self->_env, self->_error));
4275+
OnError(Error::New(_env, _error));
42654276
}
42664277
return nullptr;
42674278
});
42684279
}
4269-
if (!self->_suppress_destruct) {
4270-
self->Destroy();
4280+
if (!_suppress_destruct) {
4281+
Destroy();
42714282
}
42724283
}
42734284

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

46334644
template<class T>
46344645
inline AsyncProgressWorker<T>::AsyncProgressWorker(const Object& receiver,
4635-
const Function& callback)
4646+
const Function& callback)
46364647
: AsyncProgressWorker(receiver, callback, "generic") {
46374648
}
46384649

46394650
template<class T>
46404651
inline AsyncProgressWorker<T>::AsyncProgressWorker(const Object& receiver,
4641-
const Function& callback,
4642-
const char* resource_name)
4652+
const Function& callback,
4653+
const char* resource_name)
46434654
: AsyncProgressWorker(receiver,
46444655
callback,
46454656
resource_name,
@@ -4665,14 +4676,14 @@ inline AsyncProgressWorker<T>::AsyncProgressWorker(Napi::Env env)
46654676

46664677
template<class T>
46674678
inline AsyncProgressWorker<T>::AsyncProgressWorker(Napi::Env env,
4668-
const char* resource_name)
4679+
const char* resource_name)
46694680
: AsyncProgressWorker(env, resource_name, Object::New(env)) {
46704681
}
46714682

46724683
template<class T>
46734684
inline AsyncProgressWorker<T>::AsyncProgressWorker(Napi::Env env,
4674-
const char* resource_name,
4675-
const Object& resource)
4685+
const char* resource_name,
4686+
const Object& resource)
46764687
: AsyncWorker(env, resource_name, resource),
46774688
_asyncdata(nullptr),
46784689
_asyncsize(0) {
@@ -4751,7 +4762,6 @@ template<class T>
47514762
inline void AsyncProgressWorker<T>::ExecutionProgress::Send(const T* data, size_t count) const {
47524763
_worker->SendProgress_(data, count);
47534764
}
4754-
47554765
#endif
47564766

47574767
////////////////////////////////////////////////////////////////////////////////

‎napi.h‎

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1986,6 +1986,10 @@ namespace Napi {
19861986
ObjectReference& Receiver();
19871987
FunctionReference& Callback();
19881988

1989+
virtual void OnExecute(Napi::Env env);
1990+
virtual void OnWorkComplete(Napi::Env env,
1991+
napi_status status);
1992+
19891993
protected:
19901994
explicit AsyncWorker(const Function& callback);
19911995
explicit AsyncWorker(const Function& callback,
@@ -2019,10 +2023,10 @@ namespace Napi {
20192023
void SetError(const std::string& error);
20202024

20212025
private:
2022-
static void OnExecute(napi_env env, void* this_pointer);
2023-
static void OnWorkComplete(napi_env env,
2024-
napi_status status,
2025-
void* this_pointer);
2026+
static inline void OnAsyncWorkExecute(napi_env env, void* asyncworker);
2027+
static inline void OnAsyncWorkComplete(napi_env env,
2028+
napi_status status,
2029+
void* asyncworker);
20262030

20272031
napi_env _env;
20282032
napi_async_work _work;
@@ -2254,33 +2258,32 @@ namespace Napi {
22542258
};
22552259

22562260
protected:
2257-
explicit AsyncProgressWorker(const Function& callback);
2258-
explicit AsyncProgressWorker(const Function& callback,
2259-
const char* resource_name);
2260-
explicit AsyncProgressWorker(const Function& callback,
2261-
const char* resource_name,
2262-
const Object& resource);
2263-
explicit AsyncProgressWorker(const Object& receiver,
2264-
const Function& callback);
2265-
explicit AsyncProgressWorker(const Object& receiver,
2266-
const Function& callback,
2267-
const char* resource_name);
2268-
explicit AsyncProgressWorker(const Object& receiver,
2269-
const Function& callback,
2270-
const char* resource_name,
2271-
const Object& resource);
2261+
explicit AsyncProgressWorker(const Function& callback);
2262+
explicit AsyncProgressWorker(const Function& callback,
2263+
const char* resource_name);
2264+
explicit AsyncProgressWorker(const Function& callback,
2265+
const char* resource_name,
2266+
const Object& resource);
2267+
explicit AsyncProgressWorker(const Object& receiver,
2268+
const Function& callback);
2269+
explicit AsyncProgressWorker(const Object& receiver,
2270+
const Function& callback,
2271+
const char* resource_name);
2272+
explicit AsyncProgressWorker(const Object& receiver,
2273+
const Function& callback,
2274+
const char* resource_name,
2275+
const Object& resource);
22722276

22732277
// Optional callback of Napi::ThreadSafeFunction only available after NAPI_VERSION 4.
22742278
// Refs: https://github.com/nodejs/node/pull/27791
22752279
#if NAPI_VERSION > 4
2276-
explicit AsyncProgressWorker(Napi::Env env);
2277-
explicit AsyncProgressWorker(Napi::Env env,
2278-
const char* resource_name);
2279-
explicit AsyncProgressWorker(Napi::Env env,
2280-
const char* resource_name,
2281-
const Object& resource);
2280+
explicit AsyncProgressWorker(Napi::Env env);
2281+
explicit AsyncProgressWorker(Napi::Env env,
2282+
const char* resource_name);
2283+
explicit AsyncProgressWorker(Napi::Env env,
2284+
const char* resource_name,
2285+
const Object& resource);
22822286
#endif
2283-
22842287
virtual void Execute(const ExecutionProgress& progress) = 0;
22852288
virtual void OnProgress(const T* data, size_t count) = 0;
22862289

0 commit comments

Comments
 (0)