Skip to content

Commit

Permalink
AsyncWorker: introduce Destroy() method
Browse files Browse the repository at this point in the history
`AsyncWorker` contained the assumption that instances of its subclasses
were allocated using `new`, because it unconditionally destroyed
instances using `delete`.

This change replaces the call to `delete` with a call to a protected
instance method `Destroy()`, which can be overridden by subclasses.
This ensures that users can employ their own allocators when
creating `AsyncWorker` subclass instances because they can override
the `Destroy()` method to use their deallocator of choice.

Re: nodejs/node-addon-api#231 (comment)
PR-URL: nodejs/node-addon-api#488
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
John French committed Jun 11, 2019
1 parent 16a1fa7 commit bf4ebda
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 1 deletion.
13 changes: 13 additions & 0 deletions doc/async_worker.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,19 @@ class was created, passing in the error as the first parameter.
virtual void Napi::AsyncWorker::OnError(const Napi::Error& e);
```
### Destroy
This method is invoked when the instance must be deallocated. If
`SuppressDestruct()` was not called then this method will be called after either
`OnError()` or `OnOK()` complete. The default implementation of this method
causes the instance to delete itself using the `delete` operator. The method is
provided so as to ensure that instances allocated by means other than the `new`
operator can be deallocated upon work completion.
```cpp
virtual void Napi::AsyncWorker::Destroy();
```

### Constructor

Creates a new `Napi::AsyncWorker`.
Expand Down
6 changes: 5 additions & 1 deletion napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3529,6 +3529,10 @@ inline AsyncWorker::~AsyncWorker() {
}
}

inline void AsyncWorker::Destroy() {
delete this;
}

inline AsyncWorker::AsyncWorker(AsyncWorker&& other) {
_env = other._env;
other._env = nullptr;
Expand Down Expand Up @@ -3623,7 +3627,7 @@ inline void AsyncWorker::OnWorkComplete(
});
}
if (!self->_suppress_destruct) {
delete self;
self->Destroy();
}
}

Expand Down
1 change: 1 addition & 0 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -1803,6 +1803,7 @@ namespace Napi {
virtual void Execute() = 0;
virtual void OnOK();
virtual void OnError(const Error& e);
virtual void Destroy();

void SetError(const std::string& error);

Expand Down

0 comments on commit bf4ebda

Please sign in to comment.