-
-
Notifications
You must be signed in to change notification settings - Fork 34.3k
node-api: use v-table to reverse module dependencies #60916
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
base: main
Are you sure you want to change the base?
Conversation
|
Review requested:
|
|
From the PR description this seems super valuable and a great built-in alternative to the weak-node-api library we've been working on to add Node-API support to React Native. As such, I'd be happy for us to adopt this approach over the stuff we have now 👍 The limitation around the module being bound to a single runtime, might not be an issue as a multi-runtime host can inject functions that deal with that internally. I'm left wondering how (if at all) add-ons which are statically linked into the process are affected by this proposal? I guess not at all, as they can still register themselves and call into the global Node-API functions resolved at link time. |
|
nice! node-api symbol visibility has been a pain for us in deno so we'd love to adopt this approach as well. |
| NULL, \ | ||
| {0}, \ | ||
| }; \ | ||
| NAPI_C_CTOR(_register_##modname) { napi_module_register(&_module); } \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should still support addons compiled with legacy Node-API headers. Could this new v-table approach be opt-in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, we should make it opt-in to start with and play with it a bit.
If it works well, then we can promote it to be default at some point.
I considered to use the NAPI_EXPERIMENTAL, but it seems it is not the right approach since we use the NAPI_EXPERIMENTAL for new Node-API functions, and the module loading is an orthogonal process. Thus, a special opt-in flag would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a new macro NODE_API_MODULE_USE_VTABLE. It is currently used only by two tests.
| #define NAPI_NO_RETURN | ||
| #endif | ||
|
|
||
| // Used by deprecated registration method napi_module_register. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For anyone wondering, this was moved into the node_api_types.h.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to move it because the new vtable struct uses it.
|
There might be a potential a potential coordination problem, where the addon tries to initialize itself before the vtable gets injected (set) by the host. I don't see that as an issue when the addon relies on "symbol based" module registration, since the host can ensure to call the initialize function after setting the vtable, but how about an addon trying to call |
these should probably be exclusive modes, so napi_module_register wouldn't be called, or could only be called from within the host call to the inject function. |
src/node_api.cc
Outdated
|
|
||
| #endif | ||
|
|
||
| node_api_vtable g_vtable = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be const?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I will fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added const for the global variables related to the v-tables.
The I am going to add a conditional flag and restore the deleted test as @legendecas suggested. This test is using the |
Right, I would expect it too, but we should verify and test this scenario. |
|
The issues that I am facing on Mac and Linux are due to the use of real "C" compilers where the |
|
Nice!👍 In Lynx/PrimJS, we are currently using a similar vtable approach to address the needs of multi-runtime injection. However, our previous API was not fully aligned with the Node-API standard, which is a problem I have been working to fix recently. |
cf9b056 to
f3d584b
Compare
It is great to hear it! Any suggestions to improve code in this PR to fit your scenario are welcome. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #60916 +/- ##
==========================================
- Coverage 88.53% 88.45% -0.08%
==========================================
Files 704 704
Lines 208759 208899 +140
Branches 40281 40393 +112
==========================================
- Hits 184816 184787 -29
+ Misses 15947 15929 -18
- Partials 7996 8183 +187
🚀 New features to boost your workflow:
|
| "target_name": "binding", | ||
| "sources": [ "binding.c" ] | ||
| "sources": [ "binding.c" ], | ||
| 'defines': [ 'NODE_API_MODULE_USE_VTABLE' ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add a new target named binding_vtable and verify that the hello_world test can be kept and run in the same process with the new vtable binding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the suggestion! I had duplicated all the targets and added there the "_vtable" suffix.
src/node_api.h
Outdated
|
|
||
| #ifdef NODE_API_MODULE_USE_VTABLE | ||
| #define NODE_API_MODULE_SET_VTABLE_DEFINITION \ | ||
| const node_api_module_vtable* g_node_api_module_vtable = \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a test that contains two compilation units and this global variable can be properly defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All tests are changed to build addons with and without optional support for v-table. They show that the tests can run in both modes. In one case node-api/1_hello_world I have added a test that supports v-table, but does not implement a fallback implementation when it is missing. It passes in new version of Node.js, but fails in old version.
All other tests pass in the old versions on Node.sj except for the cases when the tests are using APIs that are not part of the previous Node.js version, e.g. Float16.
IMHO, we should run the Node-API tests against the new and old Node.js versions, but it is a subject for another discussion.
|
👍 From ABI stability, adding new function pointers at the end of the struct/V-table is ABI-stable. Lastly, there are some runtimes that support multiple JS engines, and they would need to dispatch to the correct V-table (based on the What might be good to consider is adding a function that returns a V-table for a given version. This way, if a Node Addon is using say N-API version 8, and the runtime supports, say, N-API version 10, such edit: If we can bump |
713c148 to
24b85d1
Compare
@mani3xis , thank you for the feedback and great ideas! In the last few weeks I was working on improving the code and was able to achieve good results that should now enable multiple runtimes in the same executable since the v-table is now exposed from the |
9d095bf to
d3bd284
Compare
|
@vmoroz Hi, Could I experimentally try out the solution proposed in this PR in my project first? I’ve been struggling with symbol conflicts between multiple NAPI implementations lately (callstackincubator/react-native-node-api#341), and this solution seems like it could fix my problem—I can’t wait to give it a shot. |
Hi @RobinWuu , yes, feel free to try it. I am curious to know if it works for you. At this point I consider the implementation to be quite complete. Though it is still a subject of a code review, addressing feedback, etc. I cannot predict when and if at all it will be merged to Node.js project. I.e. there is some risk in using it at the early stage. |
Summary
This PR reverses how Node-API modules bind to runtime functions. Currently, modules depend on Node-API symbols exported by the runtime (e.g.,
node.exe). With this change, the runtime instead provides Node-API functions to modules via vtables—structs containing function pointers exposed throughnapi_envand other previously opaque stracts.This enables runtime-agnostic modules: the same pre-built
.nodemodule can be loaded by any Node-API-compatible runtime without recompilation or platform-specific binding hooks. This includes standalone runtimes, embedded scenarios (e.g.,libnodeinside an application), and even applications hosting multiple embedded JS runtimes simultaneously—each runtime provides its own vtables to the modules it loads.The Issue
The current Node-API module binding has platform-specific limitations (see abi-stable-node#471):
win_delay_load_hook.ccto resolve symbols fromlibnode.dllor the current process. Modules compiled without this hook cannot load in alternative runtimes. Even with the hook, it cannot route to other embedded Node-API runtimes besideslibnode.dll.The Solution
Reverse the dependency: instead of modules binding to runtime symbols, the runtime exposes its API through vtables in
napi_envand related handles.Design
Two vtables:
node_api_js_vtable(~120 functions fromjs_native_api.h) andnode_api_module_vtable(~40 functions fromnode_api.h). Separated becausejs_native_api.hcan be used independently of Node.js-specific APIs.Exposed via structs: Three previously opaque structs now have concrete definitions with vtable pointers:
Similarly for
napi_threadsafe_function__andnapi_async_cleanup_hook_handle__(for functions which don't takenapi_envas their first argument).Sentinel detection: The sentinel is compared against
NODE_API_VT_SENTINELto detect vtable-enabled runtimes vs. legacy runtimes. Since legacynapi_env__has a C++ vtable ptr (always aligned) as the first field, the sentinel has the bit 0 set to 1 to avoid any possible matches.Opt-in: Define
NODE_API_MODULE_USE_VTABLEto enable. Node-API functions becomestatic inlinewrappers that dispatch through the vtable, with optional fallback to symbol binding for legacy runtimes.NODE_API_MODULE_USE_VTABLENODE_API_MODULE_NO_VTABLE_FALLBACKNODE_API_MODULE_NO_VTABLE_IMPLCompatibility
*Fallback can be disabled with
NODE_API_MODULE_NO_VTABLE_FALLBACK.How it works:
env->sentinel == NODE_API_VT_SENTINELdlsym/GetProcAddressand cache in a local vtableNODE_API_MODULE_USE_VTABLE) are unaffected—they continue using symbol binding as todayN.B.: Multi-runtime environments require vtable-enabled modules. Legacy modules bind to whichever runtime's symbols are visible in the process—they cannot distinguish between runtimes.
Testing
Test Infrastructure Changes
// Addons:comment in the JS file (e.g.,// Addons: binding, binding_vtable). The test runner reads this list and runs the same test for each addon variant.test/common/addon-test.jsprovidesaddonPathbased on the--addon=<name>argument, allowing one test file to run against multiple addons.Build Variants
Each test addon is compiled in multiple configurations via
binding.gyp:{ "target_name": "binding" }, // original (no vtable) { "target_name": "binding_vtable", "defines": [ "NODE_API_MODULE_USE_VTABLE" ] }, // vtable with fallbackThe
1_hello_worldtest also includes additional variants:{ "target_name": "binding_vtable_nofb", "defines": [ "NODE_API_MODULE_USE_VTABLE", "NODE_API_MODULE_NO_VTABLE_FALLBACK" ] }, // vtable only, no fallback { "target_name": "binding_vtable_noimpl", "defines": [ "NODE_API_MODULE_USE_VTABLE", "NODE_API_MODULE_NO_VTABLE_IMPL" ] }, // direct vtable accessBackward Compatibility Verification
All vtable-enabled tests (with fallback) were run against Node.js v24. Tests pass except those using APIs added after v24 (e.g., Float16). This confirms that the fallback mechanism works correctly with older runtimes.
The PR includes a number of other changes which are going to be split up into separate PRs: #61318, #61320, #61321