diff --git a/src/node_api.cc b/src/node_api.cc index 06f18562e7d3c0..3cdd4b8a970f28 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -712,6 +712,45 @@ bool FindWrapper(v8::Local obj, return true; } +static void DeleteEnv(napi_env env, void* data, void* hint) { + delete env; +} + +napi_env GetEnv(v8::Local 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("N-API Environment"), + v8::NewStringType::kInternalized).ToLocalChecked()); + auto value = global->GetPrivate(context, key).ToLocalChecked(); + + if (value->IsExternal()) { + result = static_cast(value.As()->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 @@ -723,9 +762,9 @@ void napi_module_register_cb(v8::Local exports, void* priv) { napi_module* mod = static_cast(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, diff --git a/test/addons-napi/test_env_sharing/binding.gyp b/test/addons-napi/test_env_sharing/binding.gyp new file mode 100644 index 00000000000000..5699a8391dd347 --- /dev/null +++ b/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" ] + } + ] +} diff --git a/test/addons-napi/test_env_sharing/compare_env.c b/test/addons-napi/test_env_sharing/compare_env.c new file mode 100644 index 00000000000000..b209c8f6d8e968 --- /dev/null +++ b/test/addons-napi/test_env_sharing/compare_env.c @@ -0,0 +1,22 @@ +#include +#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) diff --git a/test/addons-napi/test_env_sharing/store_env.c b/test/addons-napi/test_env_sharing/store_env.c new file mode 100644 index 00000000000000..391f6faa27648e --- /dev/null +++ b/test/addons-napi/test_env_sharing/store_env.c @@ -0,0 +1,12 @@ +#include +#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) diff --git a/test/addons-napi/test_env_sharing/test.js b/test/addons-napi/test_env_sharing/test.js new file mode 100644 index 00000000000000..6e21bf4c638b80 --- /dev/null +++ b/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');