n-api: add generic finalizer callback #22244
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nodejs-github-bot
added
c++
mhdawson
reviewed
Aug 10, 2018
mhdawson
reviewed
Aug 10, 2018
doc/api/n-api.md
Outdated
| [`napi_delete_reference`][] ONLY in response to the finalize callback | ||
| invocation. If it is deleted before then, then the finalize callback may never | ||
| be invoked. Therefore, when obtaining a reference a finalize callback is also | ||
| required in order to enable correct proper of the reference. |
There was a problem hiding this comment.
I am a bit confused here, is proper of the reference some C++ term?
|
@vsemozhetbyt @mhdawson I have now addressed your review comments. |
|
Doc LGTM. |
|
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
force-pushed
the
napi-create-dynamic-function
branch
from
5d400c1
to
b373ccf
Compare
|
@addaleax rebased. |
jasnell
approved these changes
Sep 4, 2018
BridgeAR
added
the
author ready
|
@nodejs/n-api PTAL |
|
CI resumed as: https://ci.nodejs.org/job/node-test-pull-request/17055/ |
4 tasks
|
Whops! Accidentally deleted the branch. Weird that I should have called it "napi-create-dynamic-function" O_o |
|
Another new CI: https://ci.nodejs.org/job/node-test-pull-request/17147/ |
|
Resumed CI as: https://ci.nodejs.org/job/node-test-pull-request/17153/ |
|
Landed in cf0e881. |
gabrielschulhof
pushed a commit
to gabrielschulhof/node
that referenced
this pull request
Sep 13, 2018
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 <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
targos
pushed a commit
that referenced
this pull request
Sep 14, 2018
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 <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
targos
pushed a commit
that referenced
this pull request
Sep 19, 2018
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 <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
targos
pushed a commit
that referenced
this pull request
Sep 20, 2018
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 <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
charmander
reviewed
Nov 28, 2018
| @@ -3655,6 +3671,47 @@ object `js_object` using `napi_wrap()` and removes the wrapping. If a finalize | |||
| callback was associated with the wrapping, it will no longer be called when the | |||
| JavaScript object becomes garbage-collected. | |||
|
|
|||
| ### napi_add_finalizer | |||
| <!-- YAML | |||
| added: v8.0.0 | |||
There was a problem hiding this comment.
Is v8.0.0 right? Also, should it be marked as experimental, since it needs NAPI_EXPERIMENTAL?
There was a problem hiding this comment.
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 was a problem hiding this comment.
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.
4 tasks
gabrielschulhof
pushed a commit
to gabrielschulhof/node
that referenced
this pull request
Sep 2, 2019
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 <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
BethGriggs
pushed a commit
that referenced
this pull request
Sep 19, 2019
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 <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add
napi_add_finalizer(), which provides the ability to attach datato 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 privateslot on the object, and is therefore neither removable, nor retrievable
after the call to
napi_add_finalizer(). It is assumed that the datais 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), orvcbuild test(Windows) passes