src: make a Environment-independent proxy class for NativeModuleLoader

This patch splits `NativeModuleLoader` into two parts - a singleton that only relies on v8 and `node::Mutex` and a proxy class for the singleton (`NativeModuleEnv`) that provides limited access to the singleton as well as C++ bindings for the Node.js binary. `NativeModuleLoader` is then no longer aware of `Environment`. PR-URL: #27160 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Richard Lau <riclau@uk.ibm.com>
nodejs · Apr 13, 2019 · dfd7e99 · dfd7e99
1 parent 9b6b567
commit dfd7e99
Show file tree

Hide file tree

Showing 12 changed files with 431 additions and 329 deletions.
diff --git a/node.gyp b/node.gyp
@@ -463,6 +463,7 @@
         'src/node_messaging.cc',
         'src/node_metadata.cc',
         'src/node_native_module.cc',
+        'src/node_native_module_env.cc',
         'src/node_options.cc',
         'src/node_os.cc',
         'src/node_perf.cc',
@@ -543,6 +544,7 @@
         'src/node_metadata.h',
         'src/node_mutex.h',
         'src/node_native_module.h',
+        'src/node_native_module_env.h',
         'src/node_object_wrap.h',
         'src/node_options.h',
         'src/node_options-inl.h',

diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -3,7 +3,7 @@
 #include "node_context_data.h"
 #include "node_errors.h"
 #include "node_internals.h"
-#include "node_native_module.h"
+#include "node_native_module_env.h"
 #include "node_platform.h"
 #include "node_process.h"
 #include "node_v8_platform-inl.h"
@@ -351,7 +351,7 @@ Local<Context> NewContext(Isolate* isolate,
       };
       Local<Value> arguments[] = {context->Global(), exports};
       MaybeLocal<Function> maybe_fn =
-          per_process::native_module_loader.LookupAndCompile(
+          native_module::NativeModuleEnv::LookupAndCompile(
               context, *module, &parameters, nullptr);
       if (maybe_fn.IsEmpty()) {
         return Local<Context>();

diff --git a/src/node.cc b/src/node.cc
@@ -31,7 +31,7 @@
 #include "node_errors.h"
 #include "node_internals.h"
 #include "node_metadata.h"
-#include "node_native_module.h"
+#include "node_native_module_env.h"
 #include "node_options-inl.h"
 #include "node_perf.h"
 #include "node_platform.h"
@@ -118,8 +118,10 @@
 
 namespace node {
 
+using native_module::NativeModuleEnv;
 using options_parser::kAllowedInEnvironment;
 using options_parser::kDisallowedInEnvironment;
+
 using v8::Array;
 using v8::Boolean;
 using v8::Context;
@@ -207,8 +209,7 @@ MaybeLocal<Value> ExecuteBootstrapper(Environment* env,
                                       std::vector<Local<Value>>* arguments) {
   EscapableHandleScope scope(env->isolate());
   MaybeLocal<Function> maybe_fn =
-      per_process::native_module_loader.LookupAndCompile(
-          env->context(), id, parameters, env);
+      NativeModuleEnv::LookupAndCompile(env->context(), id, parameters, env);
 
   if (maybe_fn.IsEmpty()) {
     return MaybeLocal<Value>();
@@ -401,7 +402,7 @@ MaybeLocal<Value> StartMainThreadExecution(Environment* env) {
   // To allow people to extend Node in different ways, this hook allows
   // one to drop a file lib/_third_party_main.js into the build
   // directory which will be executed instead of Node's normal loading.
-  if (per_process::native_module_loader.Exists("_third_party_main")) {
+  if (NativeModuleEnv::Exists("_third_party_main")) {
     return StartExecution(env, "internal/main/run_third_party_main");
   }
 
@@ -724,6 +725,8 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,
   per_process::metadata.versions.InitializeIntlVersions();
 #endif
 
+  NativeModuleEnv::InitializeCodeCache();
+
   // We should set node_is_initialized here instead of in node::Start,
   // otherwise embedders using node::Init to initialize everything will not be
   // able to set it and native modules will not load for them.

diff --git a/src/node_binding.cc b/src/node_binding.cc
@@ -1,8 +1,8 @@
 #include "node_binding.h"
+#include <atomic>
 #include "env-inl.h"
-#include "node_native_module.h"
+#include "node_native_module_env.h"
 #include "util.h"
-#include <atomic>
 
 #if HAVE_OPENSSL
 #define NODE_BUILTIN_OPENSSL_MODULES(V) V(crypto) V(tls_wrap)
@@ -593,13 +593,13 @@ void GetInternalBinding(const FunctionCallbackInfo<Value>& args) {
         exports->SetPrototype(env->context(), Null(env->isolate())).FromJust());
     DefineConstants(env->isolate(), exports);
   } else if (!strcmp(*module_v, "natives")) {
-    exports = per_process::native_module_loader.GetSourceObject(env->context());
+    exports = native_module::NativeModuleEnv::GetSourceObject(env->context());
     // Legacy feature: process.binding('natives').config contains stringified
     // config.gypi
     CHECK(exports
               ->Set(env->context(),
                     env->config_string(),
-                    per_process::native_module_loader.GetConfigString(
+                    native_module::NativeModuleEnv::GetConfigString(
                         env->isolate()))
               .FromJust());
   } else {

diff --git a/src/node_code_cache_stub.cc b/src/node_code_cache_stub.cc
@@ -1,5 +1,5 @@
 
-#include "node_native_module.h"
+#include "node_native_module_env.h"
 
 // This is supposed to be generated by tools/generate_code_cache.js
 // The stub here is used when configure is run without `--code-cache-path`
@@ -8,8 +8,8 @@ namespace node {
 namespace native_module {
 
 // The generated source code would insert <std::string, UnionString> pairs
-// into native_module_loader.code_cache_.
-void NativeModuleLoader::LoadCodeCache() {}
+// into NativeModuleLoader::instance.code_cache_.
+void NativeModuleEnv::InitializeCodeCache() {}
 
 }  // namespace native_module
 }  // namespace node