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

implement Object::AddFinalizer #551

Closed
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
34 changes: 34 additions & 0 deletions doc/object.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,40 @@ Returns a `bool` that is true if the `Napi::Object` is an instance created by th

Note: This is equivalent to the JavaScript instanceof operator.

### AddFinalizer()
```cpp
template <typename Finalizer, typename T>
inline void AddFinalizer(Finalizer finalizeCallback, T* data);
```

- `[in] finalizeCallback`: The function to call when the object is garbage-collected.
- `[in] data`: The data to associate with the object.

Associates `data` with the object, calling `finalizeCallback` when the object is garbage-collected. `finalizeCallback`
has the signature
```cpp
void finalizeCallback(Napi::Env env, T* data);
```
where `data` is the pointer that was passed into the call to `AddFinalizer()`.

### AddFinalizer()
```cpp
template <typename Finalizer, typename T, typename Hint>
inline void AddFinalizer(Finalizer finalizeCallback,
T* data,
Hint* finalizeHint);
```

- `[in] data`: The data to associate with the object.
- `[in] finalizeCallback`: The function to call when the object is garbage-collected.

Associates `data` with the object, calling `finalizeCallback` when the object is garbage-collected. An additional hint
may be given. It will also be passed to `finalizeCallback`, which has the signature
```cpp
void finalizeCallback(Napi::Env env, T* data, Hint* hint);
```
where `data` and `hint` are the pointers that were passed into the call to `AddFinalizer()`.

### DefineProperty()

```cpp
Expand Down
49 changes: 44 additions & 5 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,21 @@ namespace details {
template <typename FreeType>
static inline napi_status AttachData(napi_env env,
napi_value obj,
FreeType* data) {
FreeType* data,
napi_finalize finalizer = nullptr,
void* hint = nullptr) {
napi_value symbol, external;
napi_status status = napi_create_symbol(env, nullptr, &symbol);
if (status == napi_ok) {
if (finalizer == nullptr) {
finalizer = [](napi_env /*env*/, void* data, void* /*hint*/) {
delete static_cast<FreeType*>(data);
};
}
status = napi_create_external(env,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we could conditionally replace this with napi_add_finalizer if NAPI_VERSION is greater than 4?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can. I was gonna wait until v8.x drops off, but yeah, I can change it to that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I'm not sure. Let's wait until napi_add_finalizer is no longer experimental.

data,
[](napi_env /*env*/, void* data, void* /*hint*/) {
delete static_cast<FreeType*>(data);
},
nullptr,
finalizer,
hint,
&external);
if (status == napi_ok) {
napi_property_descriptor desc = {
Expand Down Expand Up @@ -1126,6 +1131,40 @@ inline bool Object::InstanceOf(const Function& constructor) const {
return result;
}

template <typename Finalizer, typename T>
inline void Object::AddFinalizer(Finalizer finalizeCallback, T* data) {
details::FinalizeData<T, Finalizer>* finalizeData =
new details::FinalizeData<T, Finalizer>({ finalizeCallback, nullptr });
napi_status status =
details::AttachData(_env,
*this,
data,
details::FinalizeData<T, Finalizer>::Wrapper,
finalizeData);
if (status != napi_ok) {
delete finalizeData;
NAPI_THROW_IF_FAILED_VOID(_env, status);
}
}

template <typename Finalizer, typename T, typename Hint>
inline void Object::AddFinalizer(Finalizer finalizeCallback,
T* data,
Hint* finalizeHint) {
details::FinalizeData<T, Finalizer, Hint>* finalizeData =
new details::FinalizeData<T, Finalizer, Hint>({ finalizeCallback, finalizeHint });
napi_status status =
details::AttachData(_env,
*this,
data,
details::FinalizeData<T, Finalizer, Hint>::WrapperWithHint,
finalizeData);
if (status != napi_ok) {
delete finalizeData;
NAPI_THROW_IF_FAILED_VOID(_env, status);
}
}

////////////////////////////////////////////////////////////////////////////////
// External class
////////////////////////////////////////////////////////////////////////////////
Expand Down
8 changes: 8 additions & 0 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,14 @@ namespace Napi {
bool InstanceOf(
const Function& constructor ///< Constructor function
) const;

template <typename Finalizer, typename T>
inline void AddFinalizer(Finalizer finalizeCallback, T* data);

template <typename Finalizer, typename T, typename Hint>
inline void AddFinalizer(Finalizer finalizeCallback,
T* data,
Hint* finalizeHint);
};

template <typename T>
Expand Down
12 changes: 11 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,17 @@
},
"directories": {},
"homepage": "https://github.com/nodejs/node-addon-api",
"keywords": ["n-api", "napi", "addon", "native", "bindings", "c", "c++", "nan", "node-addon-api"],
"keywords": [
"n-api",
"napi",
"addon",
"native",
"bindings",
"c",
"c++",
"nan",
"node-addon-api"
],
"license": "MIT",
"main": "index.js",
"name": "node-addon-api",
Expand Down
1 change: 1 addition & 0 deletions test/binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
'memory_management.cc',
'name.cc',
'object/delete_property.cc',
'object/finalizer.cc',
'object/get_property.cc',
'object/has_own_property.cc',
'object/has_property.cc',
Expand Down
1 change: 1 addition & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ let testModules = [
'memory_management',
'name',
'object/delete_property',
'object/finalizer',
'object/get_property',
'object/has_own_property',
'object/has_property',
Expand Down
29 changes: 29 additions & 0 deletions test/object/finalizer.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#include "napi.h"

using namespace Napi;

static int dummy;

Value AddFinalizer(const CallbackInfo& info) {
ObjectReference* ref = new ObjectReference;
*ref = Persistent(Object::New(info.Env()));
info[0]
.As<Object>()
.AddFinalizer([](Napi::Env /*env*/, ObjectReference* ref) {
ref->Set("finalizerCalled", true);
delete ref;
}, ref);
return ref->Value();
}

Value AddFinalizerWithHint(const CallbackInfo& info) {
ObjectReference* ref = new ObjectReference;
*ref = Persistent(Object::New(info.Env()));
info[0]
.As<Object>()
.AddFinalizer([](Napi::Env /*env*/, ObjectReference* ref, int* dummy_p) {
ref->Set("finalizerCalledWithCorrectHint", dummy_p == &dummy);
delete ref;
}, ref, &dummy);
return ref->Value();
}
21 changes: 21 additions & 0 deletions test/object/finalizer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

const buildType = process.config.target_defaults.default_configuration;
const assert = require('assert');

test(require(`../build/${buildType}/binding.node`));
test(require(`../build/${buildType}/binding_noexcept.node`));

function createWeakRef(binding, bindingToTest) {
return binding.object[bindingToTest]({});
}

function test(binding) {
const obj1 = createWeakRef(binding, 'addFinalizer');
global.gc();
assert.deepStrictEqual(obj1, { finalizerCalled: true });

const obj2 = createWeakRef(binding, 'addFinalizerWithHint');
global.gc();
assert.deepStrictEqual(obj2, { finalizerCalledWithCorrectHint: true });
}
7 changes: 7 additions & 0 deletions test/object/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ Value HasPropertyWithNapiWrapperValue(const CallbackInfo& info);
Value HasPropertyWithCStyleString(const CallbackInfo& info);
Value HasPropertyWithCppStyleString(const CallbackInfo& info);

// Native wrappers for testing Object::AddFinalizer()
Value AddFinalizer(const CallbackInfo& info);
Value AddFinalizerWithHint(const CallbackInfo& info);

static bool testValue = true;
// Used to test void* Data() integrity
struct UserDataHolder {
Expand Down Expand Up @@ -201,5 +205,8 @@ Object InitObject(Env env) {

exports["createObjectUsingMagic"] = Function::New(env, CreateObjectUsingMagic);

exports["addFinalizer"] = Function::New(env, AddFinalizer);
exports["addFinalizerWithHint"] = Function::New(env, AddFinalizerWithHint);

return exports;
}