[v8.x] n-api: add generic finalizer callback #28296
Conversation
nodejs-github-bot
added
c++
gabrielschulhof
changed the title
gabrielschulhof
force-pushed
the
backport-22244-to-v8.x
branch
from
4dbe615
to
246a6a7
Compare
There was a problem hiding this comment.
Idk … I would somehow prefer an API that would also automatically run the finalizer when the Node.js Environment exits, and while that’s totally doable, I feel like that the design of previous N-API funtions in this direction already messed that part up and so I’m not sure if it’s better to be consistent with that or better to be more easily usable here?
| CHECK_ARG(env, js_object); | ||
|
|
||
| v8::Isolate* isolate = env->isolate; | ||
| v8::Local<v8::Context> context = isolate->GetCurrentContext(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| ### napi_add_finalizer | ||
| <!-- YAML | ||
| added: v8.0.0 | ||
| napiVersion: 1 |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@addaleax the finalizer does get automatically run when the environment exits, because everything is garbage-collected at that time, right? |
|
@addaleax yikes! Looks like it's not finalizing at all on environment teardown! However, I get the impression that we can't simply bunch up all the finalizers, because there may be interdependence among them. Can we coax the garbage collector into one final pass that releases everything before the environment exits? |
No, for two reasons:
That means that the only way to actually handle Environment teardown from an addon is a cleanup hook, but since callbacks added with |
|
@addaleax OK but the finalizer added with
They may not be removable, but since they'll never be called it doesn't really matter that they're attached. We could keep a list of them in the environment, removing an item from the list when the weak callback does get called, and calling all remaining ones at environment teardown, but again, the correct order in which to call them may not necessarily be the order in which they appear in the list – though it may be OK to call them in this order – I just dunno. |
|
@addaleax I'm actually thinking that any interdependence between finalizers must already be handled by addons, because they may be called "out of order" even under normal circumstances. Like, when a database handle goes out of scope before the query handle does, or something like that. So, it may be worth bunching them up and calling any remaining ones from a cleanup hook. |
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>
gabrielschulhof
force-pushed
the
backport-22244-to-v8.x
branch
from
246a6a7
to
d3596aa
Compare
|
Rebased. |
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>
|
Landed on |
|
Backed out from |
BethGriggs
added
the
semver-minor
|
v8.x will receive no further updates. |
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
PR-URL: #22244
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes