n-api: add APIs for per-instance state management

Adds `napi_set_instance_data()` and `napi_get_instance_data()`, which allow native addons to store their data on and retrieve their data from `napi_env`. `napi_set_instance_data()` accepts a finalizer which is called when the `node::Environment()` is destroyed. This entails rendering the `napi_env` local to each add-on. Fixes: nodejs/abi-stable-node#378 PR-URL: #28682 Backport-PR-URL: #30537 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
nodejs · Feb 25, 2020 · f29fb14 · f29fb14
1 parent 20177b9
commit f29fb14
Show file tree

Hide file tree

Showing 13 changed files with 628 additions and 100 deletions.
diff --git a/doc/api/n-api.md b/doc/api/n-api.md
@@ -157,6 +157,82 @@ available to the module code.
 
 \* Indicates that the N-API version was released as experimental
 
+## Environment Life Cycle APIs
+
+> Stability: 1 - Experimental
+
+[Section 8.7][] of the [ECMAScript Language Specification][] defines the concept
+of an "Agent" as a self-contained environment in which JavaScript code runs.
+Multiple such Agents may be started and terminated either concurrently or in
+sequence by the process.
+
+A Node.js environment corresponds to an ECMAScript Agent. In the main process,
+an environment is created at startup, and additional environments can be created
+on separate threads to serve as [worker threads][]. When Node.js is embedded in
+another application, the main thread of the application may also construct and
+destroy a Node.js environment multiple times during the life cycle of the
+application process such that each Node.js environment created by the
+application may, in turn, during its life cycle create and destroy additional
+environments as worker threads.
+
+From the perspective of a native addon this means that the bindings it provides
+may be called multiple times, from multiple contexts, and even concurrently from
+multiple threads.
+
+Native addons may need to allocate global state of which they make use during
+their entire life cycle such that the state must be unique to each instance of
+the addon.
+
+To this env, N-API provides a way to allocate data such that its life cycle is
+tied to the life cycle of the Agent.
+
+### napi_set_instance_data
+<!-- YAML
+added: REPLACEME
+-->
+
+```C
+napi_status napi_set_instance_data(napi_env env,
+                                   void* data,
+                                   napi_finalize finalize_cb,
+                                   void* finalize_hint);
+```
+
+- `[in] env`: The environment that the N-API call is invoked under.
+- `[in] data`: The data item to make available to bindings of this instance.
+- `[in] finalize_cb`: The function to call when the environment is being torn
+down. The function receives `data` so that it might free it.
+- `[in] finalize_hint`: Optional hint to pass to the finalize callback
+during collection.
+
+Returns `napi_ok` if the API succeeded.
+
+This API associates `data` with the currently running Agent. `data` can later
+be retrieved using `napi_get_instance_data()`. Any existing data associated with
+the currently running Agent which was set by means of a previous call to
+`napi_set_instance_data()` will be overwritten. If a `finalize_cb` was provided
+by the previous call, it will not be called.
+
+### napi_get_instance_data
+<!-- YAML
+added: REPLACEME
+-->
+
+```C
+napi_status napi_get_instance_data(napi_env env,
+                                   void** data);
+```
+
+- `[in] env`: The environment that the N-API call is invoked under.
+- `[out] data`: The data item that was previously associated with the currently
+running Agent by a call to `napi_set_instance_data()`.
+
+Returns `napi_ok` if the API succeeded.
+
+This API retrieves data that was previously associated with the currently
+running Agent via `napi_set_instance_data()`. If no data is set, the call will
+succeed and `data` will be set to `NULL`.
+
 ## Basic N-API Data Types
 
 N-API exposes the following fundamental datatypes as abstractions that are
@@ -4735,6 +4811,7 @@ This API may only be called from the main thread.
 [Section 6.1.4]: https://tc39.github.io/ecma262/#sec-ecmascript-language-types-string-type
 [Section 6.1.6]: https://tc39.github.io/ecma262/#sec-ecmascript-language-types-number-type
 [Section 6.1.7.1]: https://tc39.github.io/ecma262/#table-2
+[Section 8.7]: https://tc39.es/ecma262/#sec-agents
 [Section 9.1.6]: https://tc39.github.io/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-defineownproperty-p-desc
 [Working with JavaScript Functions]: #n_api_working_with_javascript_functions
 [Working with JavaScript Properties]: #n_api_working_with_javascript_properties
@@ -4789,3 +4866,4 @@ This API may only be called from the main thread.
 [`uv_unref`]: http://docs.libuv.org/en/v1.x/handle.html#c.uv_unref
 [async_hooks `type`]: async_hooks.html#async_hooks_type
 [context-aware addons]: addons.html#addons_context_aware_addons
+[worker threads]: https://nodejs.org/api/worker_threads.html
diff --git a/src/env.h b/src/env.h
@@ -108,7 +108,6 @@ struct PackageConfig {
   V(contextify_context_private_symbol, "node:contextify:context")             \
   V(contextify_global_private_symbol, "node:contextify:global")               \
   V(decorated_private_symbol, "node:decorated")                               \
-  V(napi_env, "node:napi:env")                                                \
   V(napi_wrapper, "node:napi:wrapper")                                        \
   V(sab_lifetimepartner_symbol, "node:sharedArrayBufferLifetimePartner")      \
 

diff --git a/src/node_api.cc b/src/node_api.cc
@@ -25,6 +25,14 @@ struct napi_env__ {
     CHECK_NOT_NULL(node_env());
   }
 
+  virtual ~napi_env__() {
+    if (instance_data.finalize_cb != nullptr) {
+      CallIntoModuleThrow([&](napi_env env) {
+        instance_data.finalize_cb(env, instance_data.data, instance_data.hint);
+      });
+    }
+  }
+
   v8::Isolate* const isolate;  // Shortcut for context()->GetIsolate()
   node::Persistent<v8::Context> context_persistent;
 
@@ -39,11 +47,37 @@ struct napi_env__ {
   inline void Ref() { refs++; }
   inline void Unref() { if (--refs == 0) delete this; }
 
+  template <typename T, typename U>
+  void CallIntoModule(T&& call, U&& handle_exception) {
+    int open_handle_scopes_before = open_handle_scopes;
+    int open_callback_scopes_before = open_callback_scopes;
+    napi_clear_last_error(this);
+    call(this);
+    CHECK_EQ(open_handle_scopes, open_handle_scopes_before);
+    CHECK_EQ(open_callback_scopes, open_callback_scopes_before);
+    if (!last_exception.IsEmpty()) {
+      handle_exception(this, last_exception.Get(this->isolate));
+      last_exception.Reset();
+    }
+  }
+
+  template <typename T>
+  void CallIntoModuleThrow(T&& call) {
+    CallIntoModule(call, [&](napi_env env, v8::Local<v8::Value> value) {
+      env->isolate->ThrowException(value);
+    });
+  }
+
   node::Persistent<v8::Value> last_exception;
   napi_extended_error_info last_error;
   int open_handle_scopes = 0;
   int open_callback_scopes = 0;
   int refs = 1;
+  struct {
+    void* data = nullptr;
+    void* hint = nullptr;
+    napi_finalize finalize_cb = nullptr;
+  } instance_data;
 };
 
 #define NAPI_PRIVATE_KEY(context, suffix) \
@@ -158,27 +192,6 @@ struct napi_env__ {
     (out) = v8::type::New((buffer), (byte_offset), (length));                  \
   } while (0)
 
-template <typename T, typename U>
-void NapiCallIntoModule(napi_env env, T&& call, U&& handle_exception) {
-  int open_handle_scopes = env->open_handle_scopes;
-  int open_callback_scopes = env->open_callback_scopes;
-  napi_clear_last_error(env);
-  call();
-  CHECK_EQ(env->open_handle_scopes, open_handle_scopes);
-  CHECK_EQ(env->open_callback_scopes, open_callback_scopes);
-  if (!env->last_exception.IsEmpty()) {
-    handle_exception(env->last_exception.Get(env->isolate));
-    env->last_exception.Reset();
-  }
-}
-
-template <typename T>
-void NapiCallIntoModuleThrow(napi_env env, T&& call) {
-  NapiCallIntoModule(env, call, [&](v8::Local<v8::Value> value) {
-    env->isolate->ThrowException(value);
-  });
-}
-
 namespace {
 namespace v8impl {
 
@@ -357,11 +370,8 @@ class Finalizer {
   static void FinalizeBufferCallback(char* data, void* hint) {
     Finalizer* finalizer = static_cast<Finalizer*>(hint);
     if (finalizer->_finalize_callback != nullptr) {
-      NapiCallIntoModuleThrow(finalizer->_env, [&]() {
-        finalizer->_finalize_callback(
-          finalizer->_env,
-          data,
-          finalizer->_finalize_hint);
+      finalizer->_env->CallIntoModuleThrow([&](napi_env env) {
+        finalizer->_finalize_callback(env, data, finalizer->_finalize_hint);
       });
     }
 
@@ -494,12 +504,10 @@ class Reference : private Finalizer {
   static void SecondPassCallback(const v8::WeakCallbackInfo<Reference>& data) {
     Reference* reference = data.GetParameter();
 
-    napi_env env = reference->_env;
-
     if (reference->_finalize_callback != nullptr) {
-      NapiCallIntoModuleThrow(env, [&]() {
+      reference->_env->CallIntoModuleThrow([&](napi_env env) {
         reference->_finalize_callback(
-            reference->_env,
+            env,
             reference->_finalize_data,
             reference->_finalize_hint);
       });
@@ -617,7 +625,9 @@ class CallbackWrapperBase : public CallbackWrapper {
     napi_callback cb = _bundle->*FunctionField;
 
     napi_value result;
-    NapiCallIntoModuleThrow(env, [&]() { result = cb(env, cbinfo_wrapper); });
+    env->CallIntoModuleThrow([&](napi_env env) {
+      result = cb(env, cbinfo_wrapper);
+    });
 
     if (result != nullptr) {
       this->SetReturnValue(result);
@@ -781,44 +791,22 @@ v8::Local<v8::Value> CreateAccessorCallbackData(napi_env env,
 }
 
 static
-napi_env GetEnv(v8::Local<v8::Context> context) {
+napi_env NewEnv(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 value = global->GetPrivate(context, NAPI_PRIVATE_KEY(context, env))
-      .ToLocalChecked();
-
-  if (value->IsExternal()) {
-    result = static_cast<napi_env>(value.As<v8::External>()->Value());
-  } else {
-    result = new napi_env__(context);
-    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, NAPI_PRIVATE_KEY(context, env), external)
-        .FromJust());
-
-    // TODO(addaleax): There was previously code that tried to delete the
-    // napi_env when its v8::Context was garbage collected;
-    // However, as long as N-API addons using this napi_env are in place,
-    // the Context needs to be accessible and alive.
-    // Ideally, we’d want an on-addon-unload hook that takes care of this
-    // once all N-API addons using this napi_env are unloaded.
-    // For now, a per-Environment cleanup hook is the best we can do.
-    result->node_env()->AddCleanupHook(
-        [](void* arg) {
-          static_cast<napi_env>(arg)->Unref();
-        },
-        static_cast<void*>(result));
-  }
+  result = new napi_env__(context);
+  // TODO(addaleax): There was previously code that tried to delete the
+  // napi_env when its v8::Context was garbage collected;
+  // However, as long as N-API addons using this napi_env are in place,
+  // the Context needs to be accessible and alive.
+  // Ideally, we'd want an on-addon-unload hook that takes care of this
+  // once all N-API addons using this napi_env are unloaded.
+  // For now, a per-Environment cleanup hook is the best we can do.
+  result->node_env()->AddCleanupHook(
+      [](void* arg) {
+        static_cast<napi_env>(arg)->Unref();
+      },
+      static_cast<void*>(result));
 
   return result;
 }
@@ -1311,10 +1299,10 @@ void napi_module_register_by_symbol(v8::Local<v8::Object> exports,
 
   // Create a new napi_env for this module or reference one if a pre-existing
   // one is found.
-  napi_env env = v8impl::GetEnv(context);
+  napi_env env = v8impl::NewEnv(context);
 
   napi_value _exports;
-  NapiCallIntoModuleThrow(env, [&]() {
+  env->CallIntoModuleThrow([&](napi_env env) {
     _exports = init(env, v8impl::JsValueFromV8LocalValue(exports));
   });
 
@@ -3941,15 +3929,9 @@ class Work : public node::AsyncResource, public node::ThreadPoolWork {
 
     CallbackScope callback_scope(this);
 
-    // We have to back up the env here because the `NAPI_CALL_INTO_MODULE` macro
-    // makes use of it after the call into the module completes, but the module
-    // may have deallocated **this**, and along with it the place where _env is
-    // stored.
-    napi_env env = _env;
-
-    NapiCallIntoModule(env, [&]() {
-      _complete(_env, ConvertUVErrorCode(status), _data);
-    }, [env](v8::Local<v8::Value> local_err) {
+    _env->CallIntoModule([&](napi_env env) {
+      _complete(env, ConvertUVErrorCode(status), _data);
+    }, [](napi_env env, v8::Local<v8::Value> local_err) {
       // If there was an unhandled exception in the complete callback,
       // report it as a fatal exception. (There is no JavaScript on the
       // callstack that can possibly handle it.)
@@ -4287,3 +4269,26 @@ napi_status napi_add_finalizer(napi_env env,
                                          finalize_hint,
                                          result);
 }
+
+napi_status napi_set_instance_data(napi_env env,
+                                   void* data,
+                                   napi_finalize finalize_cb,
+                                   void* finalize_hint) {
+  CHECK_ENV(env);
+
+  env->instance_data.data = data;
+  env->instance_data.finalize_cb = finalize_cb;
+  env->instance_data.hint = finalize_hint;
+
+  return napi_clear_last_error(env);
+}
+
+napi_status napi_get_instance_data(napi_env env,
+                                   void** data) {
+  CHECK_ENV(env);
+  CHECK_ARG(env, data);
+
+  *data = env->instance_data.data;
+
+  return napi_clear_last_error(env);
+}
diff --git a/src/node_api.h b/src/node_api.h
@@ -726,6 +726,15 @@ NAPI_EXTERN napi_status napi_get_value_bigint_words(napi_env env,
                                                     int* sign_bit,
                                                     size_t* word_count,
                                                     uint64_t* words);
+
+// Instance data
+NAPI_EXTERN napi_status napi_set_instance_data(napi_env env,
+                                               void* data,
+                                               napi_finalize finalize_cb,
+                                               void* finalize_hint);
+
+NAPI_EXTERN napi_status napi_get_instance_data(napi_env env,
+                                               void** data);
 #endif  // NAPI_EXPERIMENTAL
 
 EXTERN_C_END

diff --git a/test/addons-napi/test_env_sharing/binding.gyp b/test/addons-napi/test_env_sharing/binding.gyp
diff --git a/test/addons-napi/test_env_sharing/test.js b/test/addons-napi/test_env_sharing/test.js
diff --git a/test/addons-napi/test_instance_data/binding.gyp b/test/addons-napi/test_instance_data/binding.gyp
@@ -0,0 +1,10 @@
+{
+  "targets": [
+    {
+      "target_name": "test_instance_data",
+      "sources": [
+        "test_instance_data.c"
+      ]
+    }
+  ]
+}