n-api: add generic finalizer callback by gabrielschulhof · Pull Request #22244 · nodejs/node

gabrielschulhof · 2018-08-10T18:03:31Z

Add napi_add_finalizer(), which provides the ability to attach data
to an arbitrary object and be notified when that object is garbage-
collected so as to have an opportunity to delete the data previously
attached.

This differs from napi_wrap() in that it does not use up the private
slot on the object, and is therefore neither removable, nor retrievable
after the call to napi_add_finalizer(). It is assumed that the data
is accessible by other means, yet it must be tied to the lifetime of
the object. This is the case for data passed to a dynamically created
function which is itself heap-allocated and must therefore be freed
along with the function.

Fixes: nodejs/abi-stable-node#313

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2018-08-10T18:03:32Z

@gabrielschulhof build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/499/pipeline

vsemozhetbyt · 2018-08-11T10:39:53Z

I am a bit confused here, is proper of the reference some C++ term?

gabrielschulhof · 2018-08-11T13:37:11Z

@vsemozhetbyt @mhdawson I have now addressed your review comments.

vsemozhetbyt · 2018-08-11T13:45:57Z

Doc LGTM.

addaleax · 2018-09-02T12:45:56Z

This would need a rebase.

Add `napi_add_finalizer()`, which provides the ability to attach data to an arbitrary object and be notified when that object is garbage- collected so as to have an opportunity to delete the data previously attached. This differs from `napi_wrap()` in that it does not use up the private slot on the object, and is therefore neither removable, nor retrievable after the call to `napi_add_finalizer()`. It is assumed that the data is accessible by other means, yet it must be tied to the lifetime of the object. This is the case for data passed to a dynamically created function which is itself heap-allocated and must therefore be freed along with the function. Fixes: nodejs/abi-stable-node#313

gabrielschulhof · 2018-09-03T14:19:54Z

@addaleax rebased.

BridgeAR · 2018-09-06T00:11:21Z

CI https://ci.nodejs.org/job/node-test-pull-request/17045/

BridgeAR · 2018-09-06T00:11:56Z

@nodejs/n-api PTAL

mhdawson

LGTM

gabrielschulhof · 2018-09-06T14:31:49Z

CI resumed as: https://ci.nodejs.org/job/node-test-pull-request/17055/

gabrielschulhof · 2018-09-09T15:10:55Z

Whops! Accidentally deleted the branch. Weird that I should have called it "napi-create-dynamic-function" O_o

gabrielschulhof · 2018-09-12T12:55:18Z

New CI: https://ci.nodejs.org/job/node-test-pull-request/17137/

gabrielschulhof · 2018-09-12T14:25:33Z

Another new CI: https://ci.nodejs.org/job/node-test-pull-request/17147/

gabrielschulhof · 2018-09-12T18:41:58Z

Resumed CI as: https://ci.nodejs.org/job/node-test-pull-request/17153/

gabrielschulhof · 2018-09-13T02:38:54Z

Landed in cf0e881.

Add `napi_add_finalizer()`, which provides the ability to attach data to an arbitrary object and be notified when that object is garbage- collected so as to have an opportunity to delete the data previously attached. This differs from `napi_wrap()` in that it does not use up the private slot on the object, and is therefore neither removable, nor retrievable after the call to `napi_add_finalizer()`. It is assumed that the data is accessible by other means, yet it must be tied to the lifetime of the object. This is the case for data passed to a dynamically created function which is itself heap-allocated and must therefore be freed along with the function. Fixes: nodejs/abi-stable-node#313 PR-URL: nodejs#22244 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Add `napi_add_finalizer()`, which provides the ability to attach data to an arbitrary object and be notified when that object is garbage- collected so as to have an opportunity to delete the data previously attached. This differs from `napi_wrap()` in that it does not use up the private slot on the object, and is therefore neither removable, nor retrievable after the call to `napi_add_finalizer()`. It is assumed that the data is accessible by other means, yet it must be tied to the lifetime of the object. This is the case for data passed to a dynamically created function which is itself heap-allocated and must therefore be freed along with the function. Fixes: nodejs/abi-stable-node#313 PR-URL: #22244 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

charmander · 2018-11-28T12:14:03Z


+### napi_add_finalizer
+<!-- YAML
+added: v8.0.0


Is v8.0.0 right? Also, should it be marked as experimental, since it needs NAPI_EXPERIMENTAL?

It does need the indicator that it is experimental. bit confusing that it says N-API version 1 even in the existing docs.https://nodejs.org/docs/latest/api/n-api.html#n_api_napi_add_finalizer. That seems wrong.

There are a few like that in the master docs, and some that don't likst the N-API version at all for the ones that are experimental. Leaving out I think is the right answer. I'll create a PR to do that in master.

Add `napi_add_finalizer()`, which provides the ability to attach data to an arbitrary object and be notified when that object is garbage- collected so as to have an opportunity to delete the data previously attached. This differs from `napi_wrap()` in that it does not use up the private slot on the object, and is therefore neither removable, nor retrievable after the call to `napi_add_finalizer()`. It is assumed that the data is accessible by other means, yet it must be tied to the lifetime of the object. This is the case for data passed to a dynamically created function which is itself heap-allocated and must therefore be freed along with the function. Fixes: nodejs/abi-stable-node#313 PR-URL: nodejs#22244 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Add `napi_add_finalizer()`, which provides the ability to attach data to an arbitrary object and be notified when that object is garbage- collected so as to have an opportunity to delete the data previously attached. This differs from `napi_wrap()` in that it does not use up the private slot on the object, and is therefore neither removable, nor retrievable after the call to `napi_add_finalizer()`. It is assumed that the data is accessible by other means, yet it must be tied to the lifetime of the object. This is the case for data passed to a dynamically created function which is itself heap-allocated and must therefore be freed along with the function. Fixes: nodejs/abi-stable-node#313 PR-URL: #22244 Backport-PR-URL: #28296 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API. labels Aug 10, 2018

mhdawson reviewed Aug 10, 2018
View reviewed changes

Comment threadtest/addons-napi/test_general/test_general.c Outdated

mhdawson reviewed Aug 10, 2018
View reviewed changes

Comment threadtest/addons-napi/test_general/testFinalizer.js Outdated

vsemozhetbyt reviewed Aug 11, 2018
View reviewed changes

gabrielschulhof force-pushed the napi-create-dynamic-function branch from 5d400c1 to b373ccfCompare September 3, 2018 13:54

jasnell approved these changes Sep 4, 2018
View reviewed changes

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 6, 2018

mhdawson approved these changes Sep 6, 2018
View reviewed changes

gabrielschulhof closed this Sep 6, 2018

gabrielschulhof deleted the napi-create-dynamic-function branch September 6, 2018 15:21

gabrielschulhof mentioned this pull request Sep 8, 2018
Fix memory-related TODOs nodejs/node-addon-api#333
Closed
4 tasks

gabrielschulhof restored the napi-create-dynamic-function branch September 9, 2018 15:09

gabrielschulhof reopened this Sep 9, 2018

gabrielschulhof closed this Sep 13, 2018

gabrielschulhof deleted the napi-create-dynamic-function branch September 13, 2018 02:39

targos mentioned this pull request Sep 18, 2018
Release proposal: v10.11.0 #22932
Merged

charmander reviewed Nov 28, 2018
View reviewed changes

gabrielschulhof mentioned this pull request Jun 19, 2019
[v8.x] n-api: add generic finalizer callback #28296
Closed
4 tasks

gabrielschulhof mentioned this pull request Jul 8, 2019
What should go into N-API 5? nodejs/abi-stable-node#374
Closed

Uh oh!

Uh oh!

Conversation

gabrielschulhof commented Aug 10, 2018

Checklist

Uh oh!

nodejs-github-bot commented Aug 10, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vsemozhetbytAug 11, 2018

Choose a reason for hiding this comment

Uh oh!

gabrielschulhof commented Aug 11, 2018

Uh oh!

vsemozhetbyt commented Aug 11, 2018

Uh oh!

addaleax commented Sep 2, 2018

Uh oh!

gabrielschulhof commented Sep 3, 2018

Uh oh!

BridgeAR commented Sep 6, 2018

Uh oh!

BridgeAR commented Sep 6, 2018

Uh oh!

mhdawson left a comment

Choose a reason for hiding this comment

Uh oh!

gabrielschulhof commented Sep 6, 2018

Uh oh!

gabrielschulhof commented Sep 9, 2018

Uh oh!

gabrielschulhof commented Sep 12, 2018

Uh oh!

gabrielschulhof commented Sep 12, 2018

Uh oh!

gabrielschulhof commented Sep 12, 2018

Uh oh!

gabrielschulhof commented Sep 13, 2018

Uh oh!

charmanderNov 28, 2018

Choose a reason for hiding this comment

Uh oh!

mhdawsonJun 20, 2019

Choose a reason for hiding this comment

Uh oh!

mhdawsonJun 20, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants