Skip to content

Commit b849b3d

Browse files
Gabriel Schulhofaddaleax
Gabriel Schulhof
authored andcommitted
n-api: re-use napi_env between modules
Store the `napi_env` on the global object at a private key. This gives us one `napi_env` per context. Refs: #14367 PR-URL: #14217 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Timothy Gu <[email protected]>
1 parent 71cb1cd commit b849b3d

File tree

5 files changed

+98
-3
lines changed

5 files changed

+98
-3
lines changed

src/node_api.cc

+42-3
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,45 @@ bool FindWrapper(v8::Local<v8::Object> obj,
707707
return true;
708708
}
709709

710+
static void DeleteEnv(napi_env env, void* data, void* hint) {
711+
delete env;
712+
}
713+
714+
napi_env GetEnv(v8::Local<v8::Context> context) {
715+
napi_env result;
716+
717+
auto isolate = context->GetIsolate();
718+
auto global = context->Global();
719+
720+
// In the case of the string for which we grab the private and the value of
721+
// the private on the global object we can call .ToLocalChecked() directly
722+
// because we need to stop hard if either of them is empty.
723+
//
724+
// Re https://github.com/nodejs/node/pull/14217#discussion_r128775149
725+
auto key = v8::Private::ForApi(isolate,
726+
v8::String::NewFromOneByte(isolate,
727+
reinterpret_cast<const uint8_t*>("N-API Environment"),
728+
v8::NewStringType::kInternalized).ToLocalChecked());
729+
auto value = global->GetPrivate(context, key).ToLocalChecked();
730+
731+
if (value->IsExternal()) {
732+
result = static_cast<napi_env>(value.As<v8::External>()->Value());
733+
} else {
734+
result = new napi_env__(isolate);
735+
auto external = v8::External::New(isolate, result);
736+
737+
// We must also stop hard if the result of assigning the env to the global
738+
// is either nothing or false.
739+
CHECK(global->SetPrivate(context, key, external).FromJust());
740+
741+
// Create a self-destructing reference to external that will get rid of the
742+
// napi_env when external goes out of scope.
743+
Reference::New(result, external, 0, true, DeleteEnv, nullptr, nullptr);
744+
}
745+
746+
return result;
747+
}
748+
710749
} // end of namespace v8impl
711750

712751
// Intercepts the Node-V8 module registration callback. Converts parameters
@@ -718,9 +757,9 @@ void napi_module_register_cb(v8::Local<v8::Object> exports,
718757
void* priv) {
719758
napi_module* mod = static_cast<napi_module*>(priv);
720759

721-
// Create a new napi_env for this module. Once module unloading is supported
722-
// we shall have to call delete on this object from there.
723-
napi_env env = new napi_env__(context->GetIsolate());
760+
// Create a new napi_env for this module or reference one if a pre-existing
761+
// one is found.
762+
napi_env env = v8impl::GetEnv(context);
724763

725764
mod->nm_register_func(
726765
env,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"targets": [
3+
{
4+
"target_name": "store_env",
5+
"sources": [ "store_env.c" ]
6+
},
7+
{
8+
"target_name": "compare_env",
9+
"sources": [ "compare_env.c" ]
10+
}
11+
]
12+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#include <node_api.h>
2+
#include "../common.h"
3+
4+
napi_value compare(napi_env env, napi_callback_info info) {
5+
napi_value external;
6+
size_t argc = 1;
7+
void* data;
8+
napi_value return_value;
9+
10+
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &external, NULL, NULL));
11+
NAPI_CALL(env, napi_get_value_external(env, external, &data));
12+
NAPI_CALL(env, napi_get_boolean(env, ((napi_env)data) == env, &return_value));
13+
14+
return return_value;
15+
}
16+
17+
void Init(napi_env env, napi_value exports, napi_value module, void* context) {
18+
napi_property_descriptor prop = DECLARE_NAPI_PROPERTY("exports", compare);
19+
NAPI_CALL_RETURN_VOID(env, napi_define_properties(env, module, 1, &prop));
20+
}
21+
22+
NAPI_MODULE(compare_env, Init)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include <node_api.h>
2+
#include "../common.h"
3+
4+
void Init(napi_env env, napi_value exports, napi_value module, void* context) {
5+
napi_value external;
6+
NAPI_CALL_RETURN_VOID(env,
7+
napi_create_external(env, env, NULL, NULL, &external));
8+
NAPI_CALL_RETURN_VOID(env,
9+
napi_set_named_property(env, module, "exports", external));
10+
}
11+
12+
NAPI_MODULE(store_env, Init)
+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
'use strict';
2+
3+
const common = require('../../common');
4+
const storeEnv = require(`./build/${common.buildType}/store_env`);
5+
const compareEnv = require(`./build/${common.buildType}/compare_env`);
6+
const assert = require('assert');
7+
8+
assert.strictEqual(compareEnv(storeEnv), true,
9+
'N-API environment pointers in two different modules have ' +
10+
'the same value');

0 commit comments

Comments
 (0)