Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add napi_finalize override to ObjectWrap #515

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -2886,6 +2886,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 @@ -3259,6 +3262,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 @@ -3400,8 +3406,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
4 changes: 3 additions & 1 deletion napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -1570,7 +1570,8 @@ namespace Napi {
class ObjectWrap : public Reference<Object> {
public:
ObjectWrap(const CallbackInfo& callbackInfo);

virtual ~ObjectWrap();

static T* Unwrap(Object wrapper);

// Methods exposed to JavaScript must conform to one of these callback signatures.
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);
mikepricedev marked this conversation as resolved.
Show resolved Hide resolved

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
21 changes: 20 additions & 1 deletion 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 @@ -197,7 +215,8 @@ const test = (binding) => {
testStaticAccessor(clazz);
testStaticMethod(clazz);

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

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