Skip to content

Commit

Permalink
src: make a Environment-independent proxy class for NativeModuleLoader
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
joyeecheung committed Apr 13, 2019
1 parent 9b6b567 commit dfd7e99
Show file tree
Hide file tree
Showing 12 changed files with 431 additions and 329 deletions.
2 changes: 2 additions & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
4 changes: 2 additions & 2 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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>();
Expand Down
11 changes: 7 additions & 4 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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>();
Expand Down Expand Up @@ -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");
}

Expand Down Expand Up @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions src/node_binding.cc
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions src/node_code_cache_stub.cc
Original file line number Diff line number Diff line change
@@ -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`
Expand All @@ -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
Loading

0 comments on commit dfd7e99

Please sign in to comment.