Skip to content
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

Allow napi_add_finalizer to work on primitive types #48311

Closed
gabrielschulhof opened this issue Jun 3, 2023 · 6 comments
Closed

Allow napi_add_finalizer to work on primitive types #48311

gabrielschulhof opened this issue Jun 3, 2023 · 6 comments
Labels
feature request Issues that request new features to be added to Node.js. node-api Issues and PRs related to the Node-API.

Comments

@gabrielschulhof
Copy link
Contributor

What is the problem this feature will solve?

We may want to have feature parity with napi_create_reference, since, internally, both create a reference.

@nodejs/node-api WDYT?

What is the feature you are proposing to solve the problem?

Open up napi_add_finalizer to primitive types.

What alternatives have you considered?

No response

@gabrielschulhof gabrielschulhof added feature request Issues that request new features to be added to Node.js. node-api Issues and PRs related to the Node-API. and removed feature request Issues that request new features to be added to Node.js. labels Jun 3, 2023
@gabrielschulhof
Copy link
Contributor Author

@vmoroz WDYT?

@gabrielschulhof gabrielschulhof added the feature request Issues that request new features to be added to Node.js. label Jun 3, 2023
@toyobayashi
Copy link
Contributor

Finalizers are implemented via FinalizationRegistry in browser, but primitive types can not work with FinalizationRegistry. When should the finalizer associated with primitive value be called?

@gabrielschulhof
Copy link
Contributor Author

@toyobayashi V8 has support for weak references to primitive types. @vmoroz implemented support for such weak references in #45715 for napi_create_reference(). I'm proposing we similarly loosen restrictions from objects-only to objects and primitive types in napi_add_finalizer().

@vmoroz
Copy link
Member

vmoroz commented Jun 5, 2023

@gabrielschulhof , I am curious how it could be implemented for the primitive types:

  • Currently the finalizers are invoked after V8 invokes them. I had an impression that V8 does it only for object types.
  • As an alternative we can notify user code when a specific napi_ref is deleted. It will have a different semantic as it will not necessarily mean that the underlying V8 object is deleted.

Do you know if V8 supports calling finalizers for primitive types?

@toyobayashi
Copy link
Contributor

toyobayashi commented Jun 6, 2023

@gabrielschulhof yes I know the implementation detail of #45715 and toyobayashi/emnapi#64 implemented it. I'm not sure if adding finalizer to primitive types could be implemented in emnapi, even if v8 supports, emnapi can't access v8 API, it relies on FinalizationRegistry to call finalizers. However FinalizationRegistry doesn't work on primitive types, so I asked when the finalizer should be called.

@gabrielschulhof
Copy link
Contributor Author

Oh, I see now. References can be created for primitive types, but not weak ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. node-api Issues and PRs related to the Node-API.
Projects
Status: Pending Triage
Development

No branches or pull requests

3 participants