-
Notifications
You must be signed in to change notification settings - Fork 461
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
Conversation
This allows one to tie the life cycle of one JavaScript object to another. In effect, this allows for JavaScript-style closures. Fixes: nodejs#508
finalizer = [](napi_env /*env*/, void* data, void* /*hint*/) { | ||
delete static_cast<FreeType*>(data); | ||
}; | ||
} | ||
status = napi_create_external(env, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
This looks good to me. I was able to make a node-addon-api decorator, which calls the provided function with the given arguments, but adds the argument length to the beginning. Value CreateDecorator(const CallbackInfo &info) {
Reference<Value> *ref = new Reference<Value>();
*ref = Persistent(info[0]);
auto f = Function::New(info.Env(), [ref](const CallbackInfo &info) -> Value {
Napi::Env env = info.Env();
auto args = std::vector<napi_value>();
args.push_back(Number::New(env, info.Length()));
for (size_t i = 0; i < info.Length(); ++i) {
args.push_back(info[i]);
}
return ref->Value().As<Function>().Call(info.This(), args);
});
f.AddFinalizer(
[](Napi::Env /*env*/, Reference<Value> *ref) {
ref->Value().As<Object>().Set("finalizerCalled", true);
delete ref;
},
ref);
return f;
} (I explicitly used I assume this is the intended API, where each scoped variable would be a reference allocated on the heap and passed in to the function, then cleared on the finalizer. |
@KevinEady I had exactly this code ready to go as a node-addon-example for after this PR landed. I guess you beat me to it 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
Would these also be called if the process exits? I tried using C++ destructors for cleanup in the past, but noticed that they only ran on GC, not on exit. |
I think it would be related to nodejs/node#22396. Finalizers would not be called on process formal exit process, neither on |
@tniessen please review nodejs/node#28428, which implements cleanup-at-environment-exit! |
Landed in bcc1d58. |
This allows one to tie the life cycle of one JavaScript object to another. In effect, this allows for JavaScript-style closures. Fixes: nodejs/node-addon-api#508 PR_URL: nodejs/node-addon-api#551 Reviewed-By: Kevin Eady <8634912+KevinEady@users.noreply.github.com>
This allows one to tie the life cycle of one JavaScript object to another. In effect, this allows for JavaScript-style closures. Fixes: nodejs/node-addon-api#508 PR_URL: nodejs/node-addon-api#551 Reviewed-By: Kevin Eady <8634912+KevinEady@users.noreply.github.com>
This allows one to tie the life cycle of one JavaScript object to another. In effect, this allows for JavaScript-style closures. Fixes: nodejs/node-addon-api#508 PR_URL: nodejs/node-addon-api#551 Reviewed-By: Kevin Eady <8634912+KevinEady@users.noreply.github.com>
This allows one to tie the life cycle of one JavaScript object to another. In effect, this allows for JavaScript-style closures. Fixes: nodejs/node-addon-api#508 PR_URL: nodejs/node-addon-api#551 Reviewed-By: Kevin Eady <8634912+KevinEady@users.noreply.github.com>
This allows one to tie the life cycle of one JavaScript object to
another. In effect, this allows for JavaScript-style closures.
Fixes: #508