Skip to content

Commit

Permalink
n-api: re-use napi_env between modules
Browse files Browse the repository at this point in the history
Store the `napi_env` on the global object at a private key. This gives
us one `napi_env` per context.

Refs: #14367
Backport-PR-URL: #19447
PR-URL: #14217
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
  • Loading branch information
Gabriel Schulhof authored and MylesBorins committed Apr 16, 2018
1 parent 06b1273 commit 86b101c
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 3 deletions.
45 changes: 42 additions & 3 deletions src/node_api.cc
Expand Up @@ -712,6 +712,45 @@ bool FindWrapper(v8::Local<v8::Object> obj,
return true;
}

static void DeleteEnv(napi_env env, void* data, void* hint) {
delete env;
}

napi_env GetEnv(v8::Local<v8::Context> context) {
napi_env result;

auto isolate = context->GetIsolate();
auto global = context->Global();

// In the case of the string for which we grab the private and the value of
// the private on the global object we can call .ToLocalChecked() directly
// because we need to stop hard if either of them is empty.
//
// Re https://github.com/nodejs/node/pull/14217#discussion_r128775149
auto key = v8::Private::ForApi(isolate,
v8::String::NewFromOneByte(isolate,
reinterpret_cast<const uint8_t*>("N-API Environment"),
v8::NewStringType::kInternalized).ToLocalChecked());
auto value = global->GetPrivate(context, key).ToLocalChecked();

if (value->IsExternal()) {
result = static_cast<napi_env>(value.As<v8::External>()->Value());
} else {
result = new napi_env__(isolate);
auto external = v8::External::New(isolate, result);

// We must also stop hard if the result of assigning the env to the global
// is either nothing or false.
CHECK(global->SetPrivate(context, key, external).FromJust());

// Create a self-destructing reference to external that will get rid of the
// napi_env when external goes out of scope.
Reference::New(result, external, 0, true, DeleteEnv, nullptr, nullptr);
}

return result;
}

} // end of namespace v8impl

// Intercepts the Node-V8 module registration callback. Converts parameters
Expand All @@ -723,9 +762,9 @@ void napi_module_register_cb(v8::Local<v8::Object> exports,
void* priv) {
napi_module* mod = static_cast<napi_module*>(priv);

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

mod->nm_register_func(
env,
Expand Down
12 changes: 12 additions & 0 deletions test/addons-napi/test_env_sharing/binding.gyp
@@ -0,0 +1,12 @@
{
"targets": [
{
"target_name": "store_env",
"sources": [ "store_env.c" ]
},
{
"target_name": "compare_env",
"sources": [ "compare_env.c" ]
}
]
}
22 changes: 22 additions & 0 deletions test/addons-napi/test_env_sharing/compare_env.c
@@ -0,0 +1,22 @@
#include <node_api.h>
#include "../common.h"

napi_value compare(napi_env env, napi_callback_info info) {
napi_value external;
size_t argc = 1;
void* data;
napi_value return_value;

NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &external, NULL, NULL));
NAPI_CALL(env, napi_get_value_external(env, external, &data));
NAPI_CALL(env, napi_get_boolean(env, ((napi_env)data) == env, &return_value));

return return_value;
}

void Init(napi_env env, napi_value exports, napi_value module, void* context) {
napi_property_descriptor prop = DECLARE_NAPI_PROPERTY("exports", compare);
NAPI_CALL_RETURN_VOID(env, napi_define_properties(env, module, 1, &prop));
}

NAPI_MODULE(compare_env, Init)
12 changes: 12 additions & 0 deletions test/addons-napi/test_env_sharing/store_env.c
@@ -0,0 +1,12 @@
#include <node_api.h>
#include "../common.h"

void Init(napi_env env, napi_value exports, napi_value module, void* context) {
napi_value external;
NAPI_CALL_RETURN_VOID(env,
napi_create_external(env, env, NULL, NULL, &external));
NAPI_CALL_RETURN_VOID(env,
napi_set_named_property(env, module, "exports", external));
}

NAPI_MODULE(store_env, Init)
10 changes: 10 additions & 0 deletions test/addons-napi/test_env_sharing/test.js
@@ -0,0 +1,10 @@
'use strict';

const common = require('../../common');
const storeEnv = require(`./build/${common.buildType}/store_env`);
const compareEnv = require(`./build/${common.buildType}/compare_env`);
const assert = require('assert');

assert.strictEqual(compareEnv(storeEnv), true,
'N-API environment pointers in two different modules have ' +
'the same value');

0 comments on commit 86b101c

Please sign in to comment.