src: define per-isolate internal bindings registration

Bindings that need to be loaded in distinct contexts of node::Realm in
the same node::Environment instance needs to share the per-isolate
templates.

Add a new binding registration callback, which is called with
per-IsolateData. This allows bindings to define templates and
share them across realms eagerly, and avoid accidental modification on
the templates when the per-context callback is called multiple times.

This also tracks loaded bindings and built-in modules with node::Realm.

PR-URL: nodejs/node#45547
Refs: nodejs/node#42528
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

Author: sercher

graal-nodejs/src/env-inl.h

@@ -778,6 +778,7 @@ void Environment::set_process_exit_handler(
 #undef VY
 #undef VP
 
+#define VM(PropertyName) V(PropertyName##_binding, v8::FunctionTemplate)
 #define V(PropertyName, TypeName)                                              \
   inline v8::Local<TypeName> IsolateData::PropertyName() const {               \
     return PropertyName##_.Get(isolate_);                                      \
@@ -786,7 +787,9 @@ void Environment::set_process_exit_handler(
     PropertyName##_.Set(isolate_, value);                                      \
   }
   PER_ISOLATE_TEMPLATE_PROPERTIES(V)
+  NODE_BINDINGS_WITH_PER_ISOLATE_INIT(VM)
 #undef V
+#undef VM
 
 #define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
 #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)

graal-nodejs/src/env.cc

@@ -311,6 +311,7 @@ IsolateDataSerializeInfo IsolateData::Serialize(SnapshotCreator* creator) {
     info.primitive_values.push_back(creator->AddData(async_wrap_provider(i)));
 
   uint32_t id = 0;
+#define VM(PropertyName) V(PropertyName##_binding, FunctionTemplate)
 #define V(PropertyName, TypeName)                                              \
   do {                                                                         \
     Local<TypeName> field = PropertyName();                                    \
@@ -321,6 +322,7 @@ IsolateDataSerializeInfo IsolateData::Serialize(SnapshotCreator* creator) {
     id++;                                                                      \
   } while (0);
   PER_ISOLATE_TEMPLATE_PROPERTIES(V)
+  NODE_BINDINGS_WITH_PER_ISOLATE_INIT(VM)
 #undef V
 
   return info;
@@ -370,6 +372,7 @@ void IsolateData::DeserializeProperties(const IsolateDataSerializeInfo* info) {
   const std::vector<PropInfo>& values = info->template_values;
   i = 0;  // index to the array
   uint32_t id = 0;
+#define VM(PropertyName) V(PropertyName##_binding, FunctionTemplate)
 #define V(PropertyName, TypeName)                                              \
   do {                                                                         \
     if (values.size() > i && id == values[i].id) {                             \
@@ -390,6 +393,7 @@ void IsolateData::DeserializeProperties(const IsolateDataSerializeInfo* info) {
   } while (0);
 
   PER_ISOLATE_TEMPLATE_PROPERTIES(V);
+  NODE_BINDINGS_WITH_PER_ISOLATE_INIT(VM);
 #undef V
 }
 
@@ -456,12 +460,12 @@ void IsolateData::CreateProperties() {
   NODE_ASYNC_PROVIDER_TYPES(V)
 #undef V
 
-  // TODO(legendecas): eagerly create per isolate templates.
   Local<FunctionTemplate> templ = FunctionTemplate::New(isolate());
   templ->InstanceTemplate()->SetInternalFieldCount(
       BaseObject::kInternalFieldCount);
   templ->Inherit(BaseObject::GetConstructorTemplate(this));
   set_binding_data_ctor_template(templ);
+  binding::CreateInternalBindingTemplates(this);
 
   contextify::ContextifyContext::InitializeGlobalTemplates(this);
 }
@@ -1585,30 +1589,13 @@ void Environment::PrintInfoForSnapshotIfDebug() {
   if (enabled_debug_list()->enabled(DebugCategory::MKSNAPSHOT)) {
     fprintf(stderr, "At the exit of the Environment:\n");
     principal_realm()->PrintInfoForSnapshot();
-    fprintf(stderr, "\nBuiltins without cache:\n");
-    for (const auto& s : builtins_without_cache) {
-      fprintf(stderr, "%s\n", s.c_str());
-    }
-    fprintf(stderr, "\nBuiltins with cache:\n");
-    for (const auto& s : builtins_with_cache) {
-      fprintf(stderr, "%s\n", s.c_str());
-    }
-    fprintf(stderr, "\nStatic bindings (need to be registered):\n");
-    for (const auto mod : internal_bindings) {
-      fprintf(stderr, "%s:%s\n", mod->nm_filename, mod->nm_modname);
-    }
   }
 }
 
 EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
   EnvSerializeInfo info;
   Local<Context> ctx = context();
 
-  // Currently all modules are compiled without cache in builtin snapshot
-  // builder.
-  info.builtins = std::vector<std::string>(builtins_without_cache.begin(),
-                                           builtins_without_cache.end());
-
   info.async_hooks = async_hooks_.Serialize(ctx, creator);
   info.immediate_info = immediate_info_.Serialize(ctx, creator);
   info.timeout_info = timeout_info_.Serialize(ctx, creator);
@@ -1660,7 +1647,6 @@ void Environment::DeserializeProperties(const EnvSerializeInfo* info) {
 
   RunDeserializeRequests();
 
-  builtins_in_snapshot = info->builtins;
   async_hooks_.Deserialize(ctx);
   immediate_info_.Deserialize(ctx);
   timeout_info_.Deserialize(ctx);
@@ -1844,8 +1830,6 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
   // Iteratable STLs have their own sizes subtracted from the parent
   // by default.
   tracker->TrackField("isolate_data", isolate_data_);
-  tracker->TrackField("builtins_with_cache", builtins_with_cache);
-  tracker->TrackField("builtins_without_cache", builtins_without_cache);
   tracker->TrackField("destroy_async_id_list", destroy_async_id_list_);
   tracker->TrackField("exec_argv", exec_argv_);
   tracker->TrackField("exiting", exiting_);

graal-nodejs/src/env.h

@@ -154,11 +154,14 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
 #undef VS
 #undef VP
 
+#define VM(PropertyName) V(PropertyName##_binding, v8::FunctionTemplate)
 #define V(PropertyName, TypeName)                                              \
   inline v8::Local<TypeName> PropertyName() const;                             \
   inline void set_##PropertyName(v8::Local<TypeName> value);
   PER_ISOLATE_TEMPLATE_PROPERTIES(V)
+  NODE_BINDINGS_WITH_PER_ISOLATE_INIT(VM)
 #undef V
+#undef VM
 
   inline v8::Local<v8::String> async_wrap_provider(int index) const;
 
@@ -178,15 +181,17 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
 #define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
 #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
 #define VS(PropertyName, StringValue) V(v8::String, PropertyName)
+#define VM(PropertyName) V(v8::FunctionTemplate, PropertyName##_binding)
 #define VT(PropertyName, TypeName) V(TypeName, PropertyName)
 #define V(TypeName, PropertyName)                                             \
   v8::Eternal<TypeName> PropertyName ## _;
   PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(VP)
   PER_ISOLATE_SYMBOL_PROPERTIES(VY)
   PER_ISOLATE_STRING_PROPERTIES(VS)
   PER_ISOLATE_TEMPLATE_PROPERTIES(VT)
+  NODE_BINDINGS_WITH_PER_ISOLATE_INIT(VM)
 #undef V
-#undef V
+#undef VM
 #undef VT
 #undef VS
 #undef VY
@@ -456,7 +461,6 @@ struct DeserializeRequest {
 };
 
 struct EnvSerializeInfo {
-  std::vector<std::string> builtins;
   AsyncHooks::SerializeInfo async_hooks;
   TickInfo::SerializeInfo tick_info;
   ImmediateInfo::SerializeInfo immediate_info;
@@ -691,13 +695,6 @@ class Environment : public MemoryRetainer {
   // List of id's that have been destroyed and need the destroy() cb called.
   inline std::vector<double>* destroy_async_id_list();
 
-  std::set<struct node_module*> internal_bindings;
-  std::set<std::string> builtins_with_cache;
-  std::set<std::string> builtins_without_cache;
-  // This is only filled during deserialization. We use a vector since
-  // it's only used for tests.
-  std::vector<std::string> builtins_in_snapshot;
-
   std::unordered_multimap<int, loader::ModuleWrap*> hash_to_module_map;
   std::unordered_map<uint32_t, loader::ModuleWrap*> id_to_module_map;
   std::unordered_map<uint32_t, contextify::ContextifyScript*>

graal-nodejs/src/node_binding.cc

@@ -110,6 +110,12 @@
 NODE_BUILTIN_BINDINGS(V)
 #undef V
 
+#define V(modname)                                                             \
+  void _register_isolate_##modname(node::IsolateData* isolate_data,            \
+                                   v8::Local<v8::FunctionTemplate> target);
+NODE_BINDINGS_WITH_PER_ISOLATE_INIT(V)
+#undef V
+
 #ifdef _AIX
 // On AIX, dlopen() behaves differently from other operating systems, in that
 // it returns unique values from each call, rather than identical values, when
@@ -237,9 +243,12 @@ static bool libc_may_be_musl() { return false; }
 namespace node {
 
 using v8::Context;
+using v8::EscapableHandleScope;
 using v8::Exception;
-using v8::Function;
 using v8::FunctionCallbackInfo;
+using v8::FunctionTemplate;
+using v8::HandleScope;
+using v8::Isolate;
 using v8::Local;
 using v8::Object;
 using v8::String;
@@ -584,50 +593,86 @@ inline struct node_module* FindModule(struct node_module* list,
   return mp;
 }
 
-static Local<Object> InitInternalBinding(Environment* env,
-                                         node_module* mod,
-                                         Local<String> module) {
-  // Internal bindings don't have a "module" object, only exports.
-  Local<Function> ctor = env->binding_data_ctor_template()
-                             ->GetFunction(env->context())
-                             .ToLocalChecked();
-  Local<Object> exports = ctor->NewInstance(env->context()).ToLocalChecked();
+void CreateInternalBindingTemplates(IsolateData* isolate_data) {
+#define V(modname)                                                             \
+  do {                                                                         \
+    Local<FunctionTemplate> templ =                                            \
+        FunctionTemplate::New(isolate_data->isolate());                        \
+    templ->InstanceTemplate()->SetInternalFieldCount(                          \
+        BaseObject::kInternalFieldCount);                                      \
+    templ->Inherit(BaseObject::GetConstructorTemplate(isolate_data));          \
+    _register_isolate_##modname(isolate_data, templ);                          \
+    isolate_data->set_##modname##_binding(templ);                              \
+  } while (0);
+  NODE_BINDINGS_WITH_PER_ISOLATE_INIT(V)
+#undef V
+}
+
+static Local<Object> GetInternalBindingExportObject(IsolateData* isolate_data,
+                                                    const char* mod_name,
+                                                    Local<Context> context) {
+  Local<FunctionTemplate> ctor;
+#define V(name)                                                                \
+  if (strcmp(mod_name, #name) == 0) {                                          \
+    ctor = isolate_data->name##_binding();                                     \
+  } else  // NOLINT(readability/braces)
+  NODE_BINDINGS_WITH_PER_ISOLATE_INIT(V)
+#undef V
+  {
+    ctor = isolate_data->binding_data_ctor_template();
+  }
+
+  Local<Object> obj = ctor->GetFunction(context)
+                          .ToLocalChecked()
+                          ->NewInstance(context)
+                          .ToLocalChecked();
+  return obj;
+}
+
+static Local<Object> InitInternalBinding(Realm* realm, node_module* mod) {
+  EscapableHandleScope scope(realm->isolate());
+  Local<Context> context = realm->context();
+  Local<Object> exports = GetInternalBindingExportObject(
+      realm->isolate_data(), mod->nm_modname, context);
   CHECK_NULL(mod->nm_register_func);
   CHECK_NOT_NULL(mod->nm_context_register_func);
-  Local<Value> unused = Undefined(env->isolate());
-  mod->nm_context_register_func(exports, unused, env->context(), mod->nm_priv);
-  return exports;
+  Local<Value> unused = Undefined(realm->isolate());
+  // Internal bindings don't have a "module" object, only exports.
+  mod->nm_context_register_func(exports, unused, context, mod->nm_priv);
+  return scope.Escape(exports);
 }
 
 void GetInternalBinding(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
+  Realm* realm = Realm::GetCurrent(args);
+  Isolate* isolate = realm->isolate();
+  HandleScope scope(isolate);
+  Local<Context> context = realm->context();
 
   CHECK(args[0]->IsString());
 
   Local<String> module = args[0].As<String>();
-  node::Utf8Value module_v(env->isolate(), module);
+  node::Utf8Value module_v(isolate, module);
   Local<Object> exports;
 
   node_module* mod = FindModule(modlist_internal, *module_v, NM_F_INTERNAL);
   if (mod != nullptr) {
-    exports = InitInternalBinding(env, mod, module);
-    env->internal_bindings.insert(mod);
+    exports = InitInternalBinding(realm, mod);
+    realm->internal_bindings.insert(mod);
   } else if (!strcmp(*module_v, "constants")) {
-    exports = Object::New(env->isolate());
-    CHECK(
-        exports->SetPrototype(env->context(), Null(env->isolate())).FromJust());
-    DefineConstants(env->isolate(), exports);
+    exports = Object::New(isolate);
+    CHECK(exports->SetPrototype(context, Null(isolate)).FromJust());
+    DefineConstants(isolate, exports);
   } else if (!strcmp(*module_v, "natives")) {
-    exports = builtins::BuiltinLoader::GetSourceObject(env->context());
+    exports = builtins::BuiltinLoader::GetSourceObject(context);
     // Legacy feature: process.binding('natives').config contains stringified
     // config.gypi
     CHECK(exports
-              ->Set(env->context(),
-                    env->config_string(),
-                    builtins::BuiltinLoader::GetConfigString(env->isolate()))
+              ->Set(context,
+                    realm->isolate_data()->config_string(),
+                    builtins::BuiltinLoader::GetConfigString(isolate))
               .FromJust());
   } else {
-    return THROW_ERR_INVALID_MODULE(env, "No such binding: %s", *module_v);
+    return THROW_ERR_INVALID_MODULE(isolate, "No such binding: %s", *module_v);
   }
 
   args.GetReturnValue().Set(exports);

graal-nodejs/src/node_binding.h

@@ -24,6 +24,8 @@ static_assert(static_cast<int>(NM_F_LINKED) ==
                   static_cast<int>(node::ModuleFlags::kLinked),
               "NM_F_LINKED != node::ModuleFlags::kLinked");
 
+#define NODE_BINDINGS_WITH_PER_ISOLATE_INIT(V) V(builtins)
+
 #define NODE_BINDING_CONTEXT_AWARE_CPP(modname, regfunc, priv, flags)          \
   static node::node_module _module = {                                         \
       NODE_MODULE_VERSION,                                                     \
@@ -51,9 +53,20 @@ node::addon_context_register_func get_node_api_context_register_func(
 
 namespace node {
 
+// Define a node internal binding that may be loaded in a context of
+// a node::Environment.
+// If an internal binding needs initializing per-isolate templates, define
+// with NODE_BINDING_PER_ISOLATE_INIT too.
 #define NODE_BINDING_CONTEXT_AWARE_INTERNAL(modname, regfunc)                  \
   NODE_BINDING_CONTEXT_AWARE_CPP(modname, regfunc, nullptr, NM_F_INTERNAL)
 
+// Define a per-isolate initialization function for a node internal binding.
+#define NODE_BINDING_PER_ISOLATE_INIT(modname, per_isolate_func)               \
+  void _register_isolate_##modname(node::IsolateData* isolate_data,            \
+                                   v8::Local<v8::FunctionTemplate> target) {   \
+    per_isolate_func(isolate_data, target);                                    \
+  }
+
 // Globals per process
 // This is set by node::Init() which is used by embedders
 extern bool node_is_initialized;
@@ -94,6 +107,8 @@ class DLib {
 // use the __attribute__((constructor)). Need to
 // explicitly call the _register* functions.
 void RegisterBuiltinBindings();
+// Create per-isolate templates for the internal bindings.
+void CreateInternalBindingTemplates(IsolateData* isolate_data);
 void GetInternalBinding(const v8::FunctionCallbackInfo<v8::Value>& args);
 void GetLinkedBinding(const v8::FunctionCallbackInfo<v8::Value>& args);
 void DLOpen(const v8::FunctionCallbackInfo<v8::Value>& args);