Skip to content

Commit

Permalink
implement virutal ObjectWrap::Finalize
Browse files Browse the repository at this point in the history
Adds instance method `Finalize()` to `ObjectWrap()` which gets called
immediately before the native instance is freed. It receives the
`Napi::Env`, so classes that override it are able to perform cleanup
that involves calls into N-API.

Re: #515 (comment)
Co-authored-by: Gabriel Schulhof <gabriel.schulhof@intel.com>
Co-authored-by: Michael Dawson <michael_dawson@ca.ibm.com>>
PR-URL: #515
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
3 people committed Jul 26, 2019
1 parent 5a7f8b2 commit c3c8814
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 1 deletion.
11 changes: 11 additions & 0 deletions doc/object_wrap.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,17 @@ property of the `Napi::CallbackInfo`.

Returns a `Napi::Function` representing the constructor function for the class.

### Finalize

Provides an opportunity to run cleanup code that requires access to the `Napi::Env`
before the wrapped native object instance is freed. Override to implement.

```cpp
virtual void Finalize(Napi::Env env);
```
- `[in] env`: `Napi::Env`.
### StaticMethod
Creates property descriptor that represents a static method of a JavaScript class.
Expand Down
9 changes: 8 additions & 1 deletion napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2888,6 +2888,9 @@ inline ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo& callbackInfo) {
*instanceRef = Reference<Object>(env, ref);
}

template<typename T>
inline ObjectWrap<T>::~ObjectWrap() {}

template<typename T>
inline T* ObjectWrap<T>::Unwrap(Object wrapper) {
T* unwrapped;
Expand Down Expand Up @@ -3261,6 +3264,9 @@ inline ClassPropertyDescriptor<T> ObjectWrap<T>::InstanceValue(
return desc;
}

template <typename T>
inline void ObjectWrap<T>::Finalize(Napi::Env /*env*/) {}

template <typename T>
inline napi_value ObjectWrap<T>::ConstructorCallbackWrapper(
napi_env env,
Expand Down Expand Up @@ -3402,8 +3408,9 @@ inline napi_value ObjectWrap<T>::InstanceSetterCallbackWrapper(
}

template <typename T>
inline void ObjectWrap<T>::FinalizeCallback(napi_env /*env*/, void* data, void* /*hint*/) {
inline void ObjectWrap<T>::FinalizeCallback(napi_env env, void* data, void* /*hint*/) {
T* instance = reinterpret_cast<T*>(data);
instance->Finalize(Napi::Env(env));
delete instance;
}

Expand Down
2 changes: 2 additions & 0 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -1570,6 +1570,7 @@ namespace Napi {
class ObjectWrap : public Reference<Object> {
public:
ObjectWrap(const CallbackInfo& callbackInfo);
virtual ~ObjectWrap();

static T* Unwrap(Object wrapper);

Expand Down Expand Up @@ -1657,6 +1658,7 @@ namespace Napi {
static PropertyDescriptor InstanceValue(Symbol name,
Napi::Value value,
napi_property_attributes attributes = napi_default);
virtual void Finalize(Napi::Env env);

private:
static napi_value ConstructorCallbackWrapper(napi_env env, napi_callback_info info);
Expand Down
17 changes: 17 additions & 0 deletions test/objectwrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ class Test : public Napi::ObjectWrap<Test> {
public:
Test(const Napi::CallbackInfo& info) :
Napi::ObjectWrap<Test>(info) {

if(info.Length() > 0) {
finalizeCb_ = Napi::Persistent(info[0].As<Napi::Function>());
}

}

void Setter(const Napi::CallbackInfo& /*info*/, const Napi::Value& value) {
Expand Down Expand Up @@ -105,8 +110,20 @@ class Test : public Napi::ObjectWrap<Test> {
}));
}

void Finalize(Napi::Env env) {

if(finalizeCb_.IsEmpty()) {
return;
}

finalizeCb_.Call(env.Global(), {Napi::Boolean::New(env, true)});
finalizeCb_.Unref();

}

private:
std::string value_;
Napi::FunctionReference finalizeCb_;
};

Napi::Object InitObjectWrap(Napi::Env env) {
Expand Down
19 changes: 19 additions & 0 deletions test/objectwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,24 @@ const test = (binding) => {
}
};

const testFinalize = (clazz) => {

let finalizeCalled = false;
const finalizeCb = function(called) {
finalizeCalled = called;
};

//Scope Test instance so that it can be gc'd.
(function() {
new Test(finalizeCb);
})();

global.gc();

assert.strictEqual(finalizeCalled, true);

};

const testObj = (obj, clazz) => {
testValue(obj, clazz);
testAccessor(obj, clazz);
Expand All @@ -198,6 +216,7 @@ const test = (binding) => {
testStaticMethod(clazz);

testStaticEnumerables(clazz);
testFinalize(clazz);
};

// `Test` is needed for accessing exposed symbols
Expand Down

0 comments on commit c3c8814

Please sign in to comment.