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

Is well-known symbol the recommended way to build addon? #21291

Closed
fs-eire opened this issue Jun 12, 2018 · 10 comments
Closed

Is well-known symbol the recommended way to build addon? #21291

fs-eire opened this issue Jun 12, 2018 · 10 comments
Labels
addons Issues and PRs related to native addons.

Comments

@fs-eire
Copy link
Contributor

fs-eire commented Jun 12, 2018

The change #18934 introduced a new interface enabling addon to initialize itself for multiple context using symbol node_register_module_v${NODE_MODULE_VERSION}.

However I didn't find much more information about this feature:
no related documents on https://nodejs.org/dist/latest-v10.x/docs/api/addons.html;
no examples ( only one simple test );
no related macro definition in node.h;
no n-api equivalent

I think the feature is still in experimental.. will it become a preferred and recommended way for addon, or it just going to be remove in some future version?

@joyeecheung joyeecheung added the addons Issues and PRs related to native addons. label Jun 12, 2018
@joyeecheung
Copy link
Member

cc @bnoordhuis @gabrielschulhof

@gabrielschulhof
Copy link
Contributor

gabrielschulhof commented Jun 12, 2018 via email

@gabrielschulhof
Copy link
Contributor

@joyeecheung I wrote the previous comment wrong. In step 1. I meant to say "... you would normally store in global static variables."

@gabrielschulhof
Copy link
Contributor

@joyeecheung AFAIK currently exports objects are never gc-ed, because we never unload modules. But then again, global static variables aren't de-allocated either, for the same reason.

@gabrielschulhof
Copy link
Contributor

gabrielschulhof commented Jun 12, 2018

@joyeecheung the N-API documentation on master mentions the macro NAPI_MODULE_INIT under the section Module registration.

@gabrielschulhof
Copy link
Contributor

This has not been back-ported though, AFAIK.

@gabrielschulhof
Copy link
Contributor

@joyeecheung the hello world addon test already tests multiple loading of an addon.

@fs-eire
Copy link
Contributor Author

fs-eire commented Jun 12, 2018

thanks @gabrielschulhof for the detailed information.

The steps show HOW to avoid using global static data, but I don't understand WHY should avoid using global static data. Sometimes I want to have a shared state (like a singleton pattern), I believe this scenario is OK to have.

So the good news is, the N-API macro NAPI_MODULE_INIT is already there ready for this kind of scenario. Is it a good idea to create a corresponding macro NODE_MODULE_INIT for the usage?

@gabrielschulhof
Copy link
Contributor

gabrielschulhof commented Jun 12, 2018

@fs-eire I guess it boils down to whether the global static singleton contains any references to JavaScript values. Since the module may be loaded multiple times from different contexts, and perhaps even different isolates, the JavaScript value(s) stored in the singleton may not be valid. Thus, if you want to have a singleton, it should only contain native data.

NODE_MODULE_INIT() may indeed be worth a PR.

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Jul 3, 2018
Introduces macros `NODE_MODULE_INITIALIZER` which expands to the name
of the special symbol that process.dlopen() will look for to initialize
an addon, and `NODE_MODULE_INIT()` which creates the boilerplate for
a context-aware module which can be loaded multiple times via the
special symbol mechanism.

Additionally, provides an example of using the new macro to construct
an addon which stores per-addon-instance data in a heap-allocated
structure that gets passed to each binding, rather than in a collection
of global static variables.

Re: nodejs#21291 (comment)
PR-URL: nodejs#21318
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
targos pushed a commit that referenced this issue Jul 4, 2018
Introduces macros `NODE_MODULE_INITIALIZER` which expands to the name
of the special symbol that process.dlopen() will look for to initialize
an addon, and `NODE_MODULE_INIT()` which creates the boilerplate for
a context-aware module which can be loaded multiple times via the
special symbol mechanism.

Additionally, provides an example of using the new macro to construct
an addon which stores per-addon-instance data in a heap-allocated
structure that gets passed to each binding, rather than in a collection
of global static variables.

Re: #21291 (comment)
PR-URL: #21318
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
@jasnell
Copy link
Member

jasnell commented Jun 19, 2020

There's been no further discussion here and it's not clear if there's anything actionable. Closing. Can reopen if necessary

@jasnell jasnell closed this as completed Jun 19, 2020
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this issue May 5, 2024
Introduces macro `NODE_MODULE_INIT()` which creates the boilerplate for
a context-aware module which can be loaded multiple times via the
special symbol mechanism.

Additionally, provides an example of using the new macro to construct
an addon which stores per-addon-instance data in a heap-allocated
structure that gets passed to each binding, rather than in a collection
of global static variables.

Re: nodejs/node#21291 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addons Issues and PRs related to native addons.
Projects
None yet
Development

No branches or pull requests

4 participants