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

No N-API equivalent for new well-known-symbol loading feature #19845

Closed
gabrielschulhof opened this issue Apr 6, 2018 · 4 comments
Closed

No N-API equivalent for new well-known-symbol loading feature #19845

gabrielschulhof opened this issue Apr 6, 2018 · 4 comments
Labels
node-api Issues and PRs related to the Node-API.

Comments

@gabrielschulhof
Copy link
Contributor

@bnoordhuis adds support for loading a module by looking for a well-known symbol if the module fails to self-register in 3828fc6. The new code looks for a symbol that matches the NODE_MODULE_VERSION (node_register_module_${NODE_MODULE_VERSION}).

We need to extend this approach to N-API modules as well.

@addaleax
Copy link
Member

addaleax commented Apr 6, 2018

Does this make sense for N-API? After all, it is supposed to be ABI-stable, so it doesn't need NODE_MODULE_VERSION... or are you suggesting to also look for node_register_module_napi?

@addaleax addaleax added the node-api Issues and PRs related to the Node-API. label Apr 6, 2018
@gabrielschulhof
Copy link
Contributor Author

@addaleax with this change, if you call dlopen() twice on the same file, that will now correctly initialize the addon twice, so this change is a step towards having multi-context native modules, in addition to being a step towards having multi-version native modules.

Fundamentally, if a plain V8 native addon fails to register, it will still have its Init called, if the Init bears the right name. Without additional work, this will not happen if it's a N-API addon.

@gabrielschulhof
Copy link
Contributor Author

@addaleax I guess the fundamental new feature here is that we now have a tool for finding a symbol given a library handle in a way that is supported across all Node.js platforms. One of my first thoughts is that we may want to

for (size_t version = NAPI_VERSION; version > 0; version--) {
  void *symbol = dlib->GetSymbolAddress((
    std::string("napi") +
    std::to_string(version) +
    "_register_module").c_str());
  if (symbol != nullptr) {
    // Initialize the N-API module
  }
}

Some outstanding questions:

  • How do we get the napi[n]_register_module symbols into the modules? Do we modify the definition of NAPI_MODULE()?
  • Since the code for looking up symbols is currently in node.cc, we need control to go over into node_api.cc somehow, because the types napi_value and napi_env are not defined in node.cc, and those are the types that need to be passed into the N-API module initializer. So, how do we get N-API to do its thing?

@gabrielschulhof
Copy link
Contributor Author

Also, #19731 may render this moot, since it allows the re-loading of modules without any special symbol. I just hope we can address all the edge cases of shared library loading in that PR.

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Apr 21, 2018
Much like regular modules, N-API modules can also benefit from having
a special symbol which they can expose.

Fixes: nodejs#19845
MylesBorins pushed a commit that referenced this issue May 4, 2018
Much like regular modules, N-API modules can also benefit from having
a special symbol which they can expose.

Fixes: #19845
PR-URL: #20161
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants