From 2c7f4f474bfbb19b7ae6597112cca41141bf71a4 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sun, 13 Jan 2019 23:44:09 +0800 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/25474 Reviewed-By: Anna Henningsen Reviewed-By: Minwoo Jung --- lib/internal/bootstrap/node.js | 54 ++++++++++++------- lib/internal/process/worker_thread_only.js | 15 +----- src/node.cc | 38 +++++++++++-- src/node_internals.h | 2 +- src/node_native_module.cc | 4 ++ src/node_native_module.h | 2 + src/node_worker.cc | 6 ++- test/message/error_exit.out | 2 +- test/message/eval_messages.out | 8 +-- .../events_unhandled_error_common_trace.out | 2 +- .../events_unhandled_error_nexttick.out | 4 +- .../events_unhandled_error_sameline.out | 4 +- test/message/nexttick_throw.out | 2 +- 13 files changed, 93 insertions(+), 50 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 14aa8d5d3203b9..53bab24ef5df75 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -301,33 +301,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 index d26159ab451c97..3fcd052542d09d 100644 --- 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 index 7a585646f66ec4..0f668472442273 100644 --- a/src/node.cc +++ b/src/node.cc @@ -647,7 +647,28 @@ static MaybeLocal 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()); } -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()); if (start_execution.IsEmpty()) return; + + Local 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. + main_script_v = Undefined(env->isolate()); + } else { + main_script_v = OneByteString(env->isolate(), main_script_id); + } + + Local 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 index 5dd0593f41dc30..70c82dd2fcd2ce 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -369,7 +369,7 @@ bool SafeGetenv(const char* key, std::string* text); void DefineZlibConstants(v8::Local 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 index 1b1a5e8ec1d860..00775885d6569c 100644 --- a/src/node_native_module.cc +++ b/src/node_native_module.cc @@ -61,6 +61,10 @@ Local ToJsSet(Local context, return out; } +bool NativeModuleLoader::Exists(const char* id) { + return source_.find(id) != source_.end(); +} + void NativeModuleLoader::GetCacheUsage( const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); diff --git a/src/node_native_module.h b/src/node_native_module.h index 9a0afcc0b96229..62c417a0b61474 100644 --- a/src/node_native_module.h +++ b/src/node_native_module.h @@ -54,6 +54,8 @@ class NativeModuleLoader { std::vector>* arguments, Environment* optional_env); + bool Exists(const char* id); + private: static void GetCacheUsage(const v8::FunctionCallbackInfo& args); // Passing ids of builtin module source code into JS land as diff --git a/src/node_worker.cc b/src/node_worker.cc index 65d3b7297cc65b..dab7945aae3551 100644 --- 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 index 6e9b42167dde18..7e0112d586cecb 100644 --- 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 index f41af4617d9dc0..3af7c9792121b9 100644 --- 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 index 67be22554fb41e..331d669272320c 100644 --- 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 index e661b32d8fb17d..8875eda532882f 100644 --- 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 index 678a9ae4b11d7d..ff024c826eb5da 100644 --- 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. (*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 index a2169b2f056fe1..4bcdcaa62c8bc5 100644 --- 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:*:*)