process: allow StartExecution() to take a main script ID

The idea is to allow the C++ layer to run arbitrary scripts as the main script. This paves the way for - cctest of the execution of Node.js instances - Earlier handling of per-process CLI options that affect execution modes (those usually do not make sense for the embedders). - Targets like mkcodecache or mksnapshot. Also moves the handling of `_third_party_main.js` into C++. PR-URL: nodejs#25474 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
BridgeAR · Jan 17, 2019 · 8390014 · 8390014
1 parent 28baf26
commit 8390014
Show file tree

Hide file tree

Showing 13 changed files with 93 additions and 50 deletions.
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -317,33 +317,47 @@ function startup() {
   } = perf.constants;
   perf.markMilestone(NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);
 
-  return startExecution;
+  if (isMainThread) {
+    return startMainThreadExecution;
+  } else {
+    return startWorkerThreadExecution;
+  }
+}
+
+function startWorkerThreadExecution() {
+  prepareUserCodeExecution();
+
+  // If we are in a worker thread, execute the script sent through the
+  // message port.
+  const {
+    getEnvMessagePort,
+    threadId
+  } = internalBinding('worker');
+  const {
+    createMessageHandler,
+    createWorkerFatalExeception
+  } = NativeModule.require('internal/process/worker_thread_only');
+
+  // Set up the message port and start listening
+  const debug = NativeModule.require('util').debuglog('worker');
+  debug(`[${threadId}] is setting up worker child environment`);
+
+  const port = getEnvMessagePort();
+  port.on('message', createMessageHandler(port));
+  port.start();
+
+  // Overwrite fatalException
+  process._fatalException = createWorkerFatalExeception(port);
 }
 
 // There are various modes that Node can run in. The most common two
 // are running from a script and running the REPL - but there are a few
 // others like the debugger or running --eval arguments. Here we decide
 // which mode we run in.
-function startExecution() {
-  // This means we are in a Worker context, and any script execution
-  // will be directed by the worker module.
-  if (!isMainThread) {
-    const workerThreadSetup = NativeModule.require(
-      'internal/process/worker_thread_only'
-    );
-    // Set up the message port and start listening
-    const { workerFatalExeception } = workerThreadSetup.setup();
-    // Overwrite fatalException
-    process._fatalException = workerFatalExeception;
-    return;
-  }
-
-  // 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 (NativeModule.exists('_third_party_main')) {
+function startMainThreadExecution(mainScript) {
+  if (mainScript) {
     process.nextTick(() => {
-      NativeModule.require('_third_party_main');
+      NativeModule.require(mainScript);
     });
     return;
   }

diff --git a/lib/internal/process/worker_thread_only.js b/lib/internal/process/worker_thread_only.js
@@ -118,19 +118,8 @@ function createWorkerFatalExeception(port) {
   };
 }
 
-function setup() {
-  debug(`[${threadId}] is setting up worker child environment`);
-
-  const port = getEnvMessagePort();
-  port.on('message', createMessageHandler(port));
-  port.start();
-
-  return {
-    workerFatalExeception: createWorkerFatalExeception(port)
-  };
-}
-
 module.exports = {
   initializeWorkerStdio,
-  setup
+  createMessageHandler,
+  createWorkerFatalExeception
 };
diff --git a/src/node.cc b/src/node.cc
@@ -647,7 +647,28 @@ static MaybeLocal<Value> ExecuteBootstrapper(
 
 void LoadEnvironment(Environment* env) {
   RunBootstrapping(env);
-  StartExecution(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")) {
+    StartExecution(env, "_third_party_main");
+  } else {
+    // TODO(joyeecheung): create different scripts for different
+    // execution modes:
+    // - `main_thread_main.js` when env->is_main_thread()
+    // - `worker_thread_main.js` when !env->is_main_thread()
+    // - `run_third_party_main.js` for `_third_party_main`
+    // - `inspect_main.js` for `node inspect`
+    // - `mkcodecache_main.js` for the code cache generator
+    // - `print_help_main.js` for --help
+    // - `bash_completion_main.js` for --completion-bash
+    // - `internal/v8_prof_processor` for --prof-process
+    // And leave bootstrap/node.js dedicated to the setup of the environment.
+    // We may want to move this switch out of LoadEnvironment, especially for
+    // the per-process options.
+    StartExecution(env, nullptr);
+  }
 }
 
 void RunBootstrapping(Environment* env) {
@@ -724,7 +745,7 @@ void RunBootstrapping(Environment* env) {
     env->set_start_execution_function(start_execution.As<Function>());
 }
 
-void StartExecution(Environment* env) {
+void StartExecution(Environment* env, const char* main_script_id) {
   HandleScope handle_scope(env->isolate());
   // We have to use Local<>::New because of the optimized way in which we access
   // the object in the env->...() getters, which does not play well with
@@ -734,8 +755,19 @@ void StartExecution(Environment* env) {
   env->set_start_execution_function(Local<Function>());
 
   if (start_execution.IsEmpty()) return;
+
+  Local<Value> main_script_v;
+  if (main_script_id == nullptr) {
+    // TODO(joyeecheung): make this mandatory - we may also create an overload
+    // for main_script that is a Local<Function>.
+    main_script_v = Undefined(env->isolate());
+  } else {
+    main_script_v = OneByteString(env->isolate(), main_script_id);
+  }
+
+  Local<Value> argv[] = {main_script_v};
   USE(start_execution->Call(
-      env->context(), Undefined(env->isolate()), 0, nullptr));
+      env->context(), Undefined(env->isolate()), arraysize(argv), argv));
 }
 
 static void StartInspector(Environment* env, const char* path) {

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -368,7 +368,7 @@ bool SafeGetenv(const char* key, std::string* text);
 void DefineZlibConstants(v8::Local<v8::Object> target);
 
 void RunBootstrapping(Environment* env);
-void StartExecution(Environment* env);
+void StartExecution(Environment* env, const char* main_script_id);
 
 }  // namespace node
 

diff --git a/src/node_native_module.cc b/src/node_native_module.cc
@@ -61,6 +61,10 @@ Local<Set> ToJsSet(Local<Context> context,
   return out;
 }
 
+bool NativeModuleLoader::Exists(const char* id) {
+  return source_.find(id) != source_.end();
+}
+
 void NativeModuleLoader::GetCacheUsage(
     const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);

diff --git a/src/node_native_module.h b/src/node_native_module.h
@@ -54,6 +54,8 @@ class NativeModuleLoader {
       std::vector<v8::Local<v8::Value>>* arguments,
       Environment* optional_env);
 
+  bool Exists(const char* id);
+
  private:
   static void GetCacheUsage(const v8::FunctionCallbackInfo<v8::Value>& args);
   // Passing map of builtin module source code into JS land as

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -185,8 +185,10 @@ void Worker::Run() {
         HandleScope handle_scope(isolate_);
         Environment::AsyncCallbackScope callback_scope(env_.get());
         env_->async_hooks()->push_async_ids(1, 0);
-        // This loads the Node bootstrapping code.
-        LoadEnvironment(env_.get());
+        RunBootstrapping(env_.get());
+        // TODO(joyeecheung): create a main script for worker threads
+        // that starts listening on the message port.
+        StartExecution(env_.get(), nullptr);
         env_->async_hooks()->pop_async_id(1);
 
         Debug(this, "Loaded environment for worker %llu", thread_id_);

diff --git a/test/message/error_exit.out b/test/message/error_exit.out
@@ -15,4 +15,4 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
diff --git a/test/message/eval_messages.out b/test/message/eval_messages.out
@@ -10,7 +10,7 @@ SyntaxError: Strict mode code may not include a with statement
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/process/execution.js:*:*)
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
 42
 42
 [eval]:1
@@ -25,7 +25,7 @@ Error: hello
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/process/execution.js:*:*)
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
 
 [eval]:1
 throw new Error("hello")
@@ -39,7 +39,7 @@ Error: hello
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/process/execution.js:*:*)
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
 100
 [eval]:1
 var x = 100; y = x;
@@ -53,7 +53,7 @@ ReferenceError: y is not defined
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     at evalScript (internal/process/execution.js:*:*)
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
 
 [eval]:1
 var ______________________________________________; throw 10

diff --git a/test/message/events_unhandled_error_common_trace.out b/test/message/events_unhandled_error_common_trace.out
@@ -19,4 +19,4 @@ Emitted 'error' event at:
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     [... lines matching original stack trace ...]
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
diff --git a/test/message/events_unhandled_error_nexttick.out b/test/message/events_unhandled_error_nexttick.out
@@ -11,11 +11,11 @@ Error
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
 Emitted 'error' event at:
     at process.nextTick (*events_unhandled_error_nexttick.js:*:*)
     at processTicksAndRejections (internal/process/next_tick.js:*:*)
     at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
diff --git a/test/message/events_unhandled_error_sameline.out b/test/message/events_unhandled_error_sameline.out
@@ -11,9 +11,9 @@ Error
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
 Emitted 'error' event at:
     at Object.<anonymous> (*events_unhandled_error_sameline.js:*:*)
     at Module._compile (internal/modules/cjs/loader.js:*:*)
     [... lines matching original stack trace ...]
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)
diff --git a/test/message/nexttick_throw.out b/test/message/nexttick_throw.out
@@ -8,4 +8,4 @@ ReferenceError: undefined_reference_error_maker is not defined
     at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:*:*)
     at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
     at executeUserCode (internal/bootstrap/node.js:*:*)
-    at startExecution (internal/bootstrap/node.js:*:*)
+    at startMainThreadExecution (internal/bootstrap/node.js:*:*)