storing of per-isolate data (#12)

* support storing per-isolate data * drop the vararg macro * avoid symbol pollution
mmomtchev · Dec 22, 2023 · 1123776 · 1123776
1 parent 00ce7c0
commit 1123776
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,8 +5,9 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-### []
+### [1.2.0]
 
+-   Support storing of a custom per-isolate data structure (`Napi::Env::GetInstanceData` and `Napi::Env::SetInstanceData`)
 -   Specify the memory ownership rules for `Buffer`
 -   Fix a minor memory leak when calling method with incorrect number of arguments
 

diff --git a/README.md b/README.md
@@ -461,15 +461,31 @@ In this case only the first argument will contain the raw V8 value.
 
 It is also possible to access the `exports` and `env` objects when initializing the module:
 ```cpp
+constexpr bool False = false;
 NOBIND_MODULE(native, m) {
-  static constexpr bool False = false;
-
   m.Exports().Set("debug_build", Napi::Boolean::New(m.Env(), true));
   m.def<Hello>("Hello")
     .def<&False, Nobind::ReadOnly>(Napi::Symbol::WellKnown(m.Env(), "isConcatSpreadable"));
 }
 ```
 
+### Storing custom per-isolate data
+
+Sometimes a module needs to store *"global"* data. With `node-addon-api` the proper way to store this data is in a per-isolate data structure - since Node.js is allowed to call the same instance from multiple independent isolates. To access the per-isolate storage with `nobind17`, declare the module specific structure and then use the standard `node-addon-api` calls to access it:
+
+```cpp
+struct PerIsolateData {
+  Napi::ObjectReference exports;
+};
+
+NOBIND_MODULE_DATA(native, m, PerIsolateData) {
+  m.Env().GetInstanceData<Nobind::EnvInstanceData<PerIsolateData>>()->exports =
+      Napi::Persistent<Napi::Object>(m.Exports());
+}
+```
+
+`nobind17` / `node-addon-api` will take care of creating and freeing this structure when new isolates are created and destroyed.
+
 ### Troubleshooting
 
 Most of the work that `nobind17` does happens during the C++ compilation of the project. It is at that moment that the templates will be instantiated.

diff --git a/include/nobind.h b/include/nobind.h
@@ -67,15 +67,20 @@ template <char const MODULE[]> class Module {
 
 } // namespace Nobind
 
-#define NOBIND_MODULE(MODULE_NAME, MODULE_ARG)                                                                         \
-  char const Nobind_##MODULE##_name[] = #MODULE_NAME;                                                                  \
-  void Nobind_##MODULE##_Init_Wrapper(Nobind::Module<Nobind_##MODULE##_name> &);                                       \
-  Napi::Object Nobind_##MODULE##_Init_Wrapper(Napi::Env, Napi::Object);                                                \
-  NODE_API_MODULE(MODULE_NAME, Nobind_##MODULE##_Init_Wrapper)                                                         \
-  Napi::Object Nobind_##MODULE##_Init_Wrapper(Napi::Env env, Napi::Object exports) {                                   \
-    env.SetInstanceData(new Nobind::EnvInstanceData);                                                                  \
-    Nobind::Module<Nobind_##MODULE##_name> m(env, exports);                                                            \
-    Nobind_##MODULE##_Init_Wrapper(m);                                                                                 \
+#define NOBIND_MODULE_DATA(MODULE_NAME, MODULE_ARG, INSTANCE_DATA_TYPE)                                                \
+  /* We make this assumption that is not guaranteed by the C++ specs but it seems to be true on all compilers */       \
+  static_assert(&Nobind::BaseEnvInstanceData::_Nobind_cons ==                                                          \
+                &Nobind::EnvInstanceData<INSTANCE_DATA_TYPE>::_Nobind_cons);                                           \
+  char const Nobind_##MODULE_NAME##_name[] = #MODULE_NAME;                                                             \
+  void Nobind_##MODULE_NAME##_Init_Wrapper(Nobind::Module<Nobind_##MODULE_NAME##_name> &);                             \
+  Napi::Object Nobind_##MODULE_NAME##_Init_Wrapper(Napi::Env, Napi::Object);                                           \
+  NODE_API_MODULE(MODULE_NAME, Nobind_##MODULE_NAME##_Init_Wrapper)                                                    \
+  Napi::Object Nobind_##MODULE_NAME##_Init_Wrapper(Napi::Env env, Napi::Object exports) {                              \
+    env.SetInstanceData(new Nobind::EnvInstanceData<INSTANCE_DATA_TYPE>);                                              \
+    Nobind::Module<Nobind_##MODULE_NAME##_name> m(env, exports);                                                       \
+    Nobind_##MODULE_NAME##_Init_Wrapper(m);                                                                            \
     return exports;                                                                                                    \
   }                                                                                                                    \
-  void Nobind_##MODULE##_Init_Wrapper(Nobind::Module<Nobind_##MODULE##_name> &MODULE_ARG)
+  void Nobind_##MODULE_NAME##_Init_Wrapper(Nobind::Module<Nobind_##MODULE_NAME##_name> &MODULE_ARG)
+
+#define NOBIND_MODULE(MODULE_NAME, MODULE_ARG) NOBIND_MODULE_DATA(MODULE_NAME, MODULE_ARG, Nobind::EmptyEnvInstanceData)
diff --git a/include/noobject.h b/include/noobject.h
@@ -10,11 +10,15 @@
 
 namespace Nobind {
 
-struct EnvInstanceData {
+struct EmptyEnvInstanceData {};
+
+struct BaseEnvInstanceData {
   // Per-environment constructors for all proxied types
-  std::vector<Napi::FunctionReference> cons;
+  std::vector<Napi::FunctionReference> _Nobind_cons;
 };
 
+template <typename T> struct EnvInstanceData : BaseEnvInstanceData, public T {};
+
 // The JS proxy object type
 template <typename CLASS> class NoObjectWrap : public Napi::ObjectWrap<NoObjectWrap<CLASS>> {
   template <typename T> friend class Typemap::FromJS;
@@ -366,8 +370,8 @@ NoObjectWrap<CLASS>::GetClass(Napi::Env env, const char *name,
 template <typename CLASS> template <bool OWNED> inline Napi::Value NoObjectWrap<CLASS>::New(Napi::Env env, CLASS *obj) {
   napi_value ext = Napi::External<CLASS>::New(env, obj);
   napi_value own = Napi::Boolean::New(env, OWNED);
-  auto instance = env.GetInstanceData<EnvInstanceData>();
-  Napi::Value r = instance->cons[class_idx].New({ext, own});
+  auto instance = env.GetInstanceData<BaseEnvInstanceData>();
+  Napi::Value r = instance->_Nobind_cons[class_idx].New({ext, own});
   return r;
 }
 
@@ -377,8 +381,8 @@ inline Napi::Value NoObjectWrap<CLASS>::New(Napi::Env env, const CLASS *obj) {
   static_assert(OWNED == false, "Cannot create an owned object from a const object, use Nobind::ReturnShared");
   napi_value ext = Napi::External<CLASS>::New(env, const_cast<CLASS *>(obj));
   napi_value own = Napi::Boolean::New(env, false);
-  auto instance = env.GetInstanceData<EnvInstanceData>();
-  Napi::Value r = instance->cons[class_idx].New({ext, own});
+  auto instance = env.GetInstanceData<BaseEnvInstanceData>();
+  Napi::Value r = instance->_Nobind_cons[class_idx].New({ext, own});
   return r;
 }
 
@@ -388,8 +392,8 @@ template <typename CLASS> inline CLASS *NoObjectWrap<CLASS>::CheckUnwrap(Napi::V
     throw Napi::TypeError::New(env, "Expected an object");
   }
   Napi::Object obj = val.ToObject();
-  auto instance = env.GetInstanceData<EnvInstanceData>();
-  if (!obj.InstanceOf(instance->cons[class_idx].Value())) {
+  auto instance = env.GetInstanceData<BaseEnvInstanceData>();
+  if (!obj.InstanceOf(instance->_Nobind_cons[class_idx].Value())) {
     throw Napi::TypeError::New(env, "Expected a " + name);
   }
   return NoObjectWrap<CLASS>::Unwrap(obj)->self;
@@ -485,9 +489,9 @@ template <class CLASS> class ClassDefinition {
 
   ~ClassDefinition() {
     Napi::Function ctor = NoObjectWrap<CLASS>::GetClass(env_, name_, properties);
-    auto instance = env_.GetInstanceData<EnvInstanceData>();
+    auto instance = env_.GetInstanceData<BaseEnvInstanceData>();
     NoObjectWrap<CLASS>::Configure(constructors, class_idx_, name_);
-    instance->cons.emplace(instance->cons.begin() + class_idx_, Napi::Persistent(ctor));
+    instance->_Nobind_cons.emplace(instance->_Nobind_cons.begin() + class_idx_, Napi::Persistent(ctor));
     exports_.Set(name_, ctor);
   }
 };

diff --git a/test/tests/native.cc b/test/tests/native.cc
@@ -11,6 +11,15 @@ Napi::Value global_native(Napi::Value val) {
   return Napi::String::New(env, r);
 }
 
+struct PerIsolateData {
+  Napi::ObjectReference exports;
+};
+
+Napi::Value get_exports(const Napi::CallbackInfo &info) {
+  Napi::Env env{info.Env()};
+  return env.GetInstanceData<Nobind::EnvInstanceData<PerIsolateData>>()->exports.Value();
+}
+
 class WithNative {
 public:
   WithNative() {}
@@ -23,10 +32,14 @@ class WithNative {
   }
 };
 
-NOBIND_MODULE(native, m) {
+NOBIND_MODULE_DATA(native, m, PerIsolateData) {
   m.def<WithNative>("WithNative").cons<>().def<&WithNative::method_native>("method_native");
   m.def<&global_native>("global_native");
 
+  m.Env().GetInstanceData<Nobind::EnvInstanceData<PerIsolateData>>()->exports =
+      Napi::Persistent<Napi::Object>(m.Exports());
+  m.Exports().Set("get_exports", Napi::Function::New(m.Env(), get_exports));
+
   m.Exports().Set("debug_build", Napi::Boolean::New(m.Env(),
 #ifdef DEBUG
                                                     true

diff --git a/test/tests/native.js b/test/tests/native.js
@@ -30,6 +30,13 @@ describe('global method', () => {
   });
 });
 
+describe('per isolate data', () => {
+  it('retrieve stored data', () => {
+    assert.isBoolean(dll.get_exports().debug_build);
+    assert.strictEqual(dll.get_exports(), dll);
+  });
+});
+
 it('manually set constant', () => {
   assert.isBoolean(dll.debug_build);
 });