Skip to content

Interoperability between the v8::External and Napi::External is broken #58480

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

Closed
NorbertHeusser opened this issue May 27, 2025 · 6 comments
Closed
Labels
node-api Issues and PRs related to the Node-API.

Comments

@NorbertHeusser
Copy link

NorbertHeusser commented May 27, 2025

Version

20.x 22.x

Platform

any

Subsystem

No response

What steps will reproduce the bug?

With the merge of PR-51149 the interoperability between the v8::External and the Napi::External API usage is broken.
Create a v8::External value using the v8-API (e.g. from the NodeJS embedder C++ API) and later retrieve the value from an native addon API base Napi::External will not result in the same native pointer.
This way it is no longer possible to provide any native data reference from the embedder API to a native addon function.

How often does it reproduce? Is there a required condition?

It is always happens. Simply create a External value based on one of the APIs and retrieve it using the other API.
E.g. create a External value from the embedder functions:

  auto cpp_object_key = v8::String::NewFromUtf8(isolate, "__my_cpp_object").ToLocalChecked();
  if (!global->Set(setup_->context(), cpp_object_key, v8::External::New(isolate, this)).FromMaybe(false)) {
    // Error handling
  }

Retrieve the pointer later from a native addon function:

  Script *script =
      env.Global().Get("__my_cpp_object")).As<Napi::External<Script>>().Data();
  if (script == nullptr) {
    // error handling
  }

What is the expected behavior? Why is that the expected behavior?

The expected behavior would be the same native pointer is given back. This worked until Node version 18.x.

With the current implementation it is no longer possible to exchange a nativ C++ pointer between code written based on the v8 API and node addon API.

In my personal view the design issue starts with the v8::External being a simple value in the V8-API. And being a TypeTaggable derived type in the addon API. With the current implementation there is no counterpart to the v8::External value type in the addon API anymore.

What do you see instead?

Creating a v8::External value and retrieve it based on the Napi::External we get a totally different pointer. The Napi::External expects the pointer to point to the internal class Napi::ExternalWrapper. But the v8::External API does not create the Napi::ExternalWrapper instance.

Additional information

We use the the Nodejs embedder API to embedd a nodejs engine into our C++ based application. As there is no node addon embedder API we have to use the node/v8 api in setting up the embedded NodeJS engine.
Additionally we provide some native addon functions to provide access to to data from the surrounding process. As we have multiple NodeJS engines running inside our process we need to find a way to provide access to the inividual C++ object creating a nodeJS instance each. Until node 18.x we used an v8::External value created in the global from the embedder function and were able to retrieve this pointer from within the addon API functions.

Workaround we use now:
The only workaround we found now is to create an v8::ArrayBuffer with the size of the native pointer, Copy the pointer into the arrray buffer. Create a v8::Uint8Array and add this array to global. In this code we have our class Script creating the embedded NodeJS engine.

  auto array_buffer = v8::ArrayBuffer::New(isolate, sizeof(Script *));
  Script *this_ptr{this};
  memcpy(array_buffer->Data(), &this_ptr, sizeof(Script *));

  auto global = setup_->context()->Global();
  auto cpp_object_key = v8::String::NewFromUtf8(isolate, "__my_cpp_script_object").ToLocalChecked();
  auto uint8_array = v8::Uint8Array::New(array_buffer, 0, sizeof(Script *));
  if (!global->Set(setup_->context(), cpp_object_key, uint8_array).FromMaybe(false)) {
    // Error handling
  }

And in the native addon function copy the pointer back from the Array to get access to the C++ Script instance:

  auto global = env.Global();
  
  auto uint8_array = global.Get("__my_cpp_script_object").As<Napi::Uint8Array>();
  Script *script{nullptr};
  memcpy(&script, uint8_array.Data(), sizeof(Script *));
@targos
Copy link
Member

targos commented May 28, 2025

@nodejs/node-api

@KevinEady
Copy link
Contributor

One could argue that the "interoperability between the v8::External and the Napi::External prior to #51149" just happened to "work" could be mere coincidence. Node-API makes no guarantees with V8 internal APIs. There is nothing within the Node-API documentation that states a Napi::External and a v8::External are 1:1. The ABI stability guarantee for Node-API is only for using Node-API within your addons, and using these APIs for a "compile once, run anywhere" across different Node.js versions (and other JavaScript engines that implement Node-API). Using Node-API is meant to circumvent the need to use V8 APIs directly.

@legendecas legendecas added the node-api Issues and PRs related to the Node-API. label May 29, 2025
@bnoordhuis
Copy link
Member

I concur. It's purely self-inflicted. Closing as a wontfix.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale May 29, 2025
@github-project-automation github-project-automation bot moved this from Need Triage to Done in Node-API Team Project May 29, 2025
@NorbertHeusser
Copy link
Author

Ok, got it. If the Node_API is not compatible with the V8-API then you might at least make the node-API feature compliant and offer some kind of embedder API as well. Right now there is no embedder API in the Node-API, which is the way why everybody embedding a nodeJS has to make use of the V8-API.
But with the broken compatiblity there is no real documented way how to access data of the embedding process through the addon functions, which you most likely have in place when embedding a NodeJS engine.

@mhdawson
Copy link
Member

@NorbertHeusser there are efforts underway support embedding in a way that would be compatible with Node-api.

See these PRs
#54660 (work on full API)
#58207 (minimal API)

Will either of these cover your use case? If not we are interested in understanding if that means there is a gap proposed APIs.

@NorbertHeusser
Copy link
Author

Thanks for guiding me to the ongoing PRs on the Node-api to offer an embedding API.
Will take a look next week and try to provide some feedback.

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
Development

No branches or pull requests

6 participants