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

test/addons-napi/test_general/test_general.c

test/addons-napi/test_general/testFinalizer.js

doc/api/n-api.md

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

doc/api/n-api.md

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

doc/api/n-api.md


+### 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

test/addons-napi/test_general/test_general.c Outdated Show resolved Hide resolved

mhdawson reviewed Aug 10, 2018

View reviewed changes

test/addons-napi/test_general/testFinalizer.js Outdated Show resolved Hide resolved

vsemozhetbyt reviewed Aug 11, 2018

View reviewed changes

gabrielschulhof force-pushed the napi-create-dynamic-function branch from 5d400c1 to b373ccf Compare 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!

Conversation

gabrielschulhof commented Aug 10, 2018

Checklist

Uh oh!

nodejs-github-bot commented Aug 10, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vsemozhetbyt Aug 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!

charmander Nov 28, 2018

Choose a reason for hiding this comment

Uh oh!

mhdawson Jun 20, 2019

Choose a reason for hiding this comment

Uh oh!

mhdawson Jun 20, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants