n-api: re-use napi_env between modules

Gabriel Schulhof · Gabriel Schulhof · commit cdb7f1341fb7 · 2017-07-22T11:46:33.000+03:00
Store the `napi_env` on the global object at a private key. This gives us one `napi_env` per context. Re: #13872 (comment)
diff --git a/src/node_api.cc b/src/node_api.cc
@@ -706,6 +706,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
@@ -717,9 +756,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,
diff --git a/test/addons-napi/test_env_sharing/binding.gyp 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
@@ -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)
diff --git a/test/addons-napi/test_env_sharing/store_env.c b/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)
diff --git a/test/addons-napi/test_env_sharing/test.js 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');
