Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 34 additions & 20 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit here is temporary - I think it would be better to do the branching in C++ and directly compileAndCall the main scripts instead. For per-process CLI options like --help we could branch in node::Start() since they do not make a lot of sense for either embedders or the workers anyway. Although unfortunately for _third_party_main we have to keep a driver that wrap it into a process.nextTick() (or maybe there is a cleverer way out?)

process.nextTick(() => {
NativeModule.require('_third_party_main');
NativeModule.require(mainScript);
});
return;
}
Expand Down
15 changes: 2 additions & 13 deletions lib/internal/process/worker_thread_only.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
38 changes: 35 additions & 3 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure I understand this correctly, overload means an overload of StartExecution() that takes a Local<Function>? I like that idea. :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addaleax Yeah, and maybe we could also use an overload with a C++ callback that takes some useful arguments (maybe Environment?)

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) {
Expand Down
2 changes: 1 addition & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,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

Expand Down
4 changes: 4 additions & 0 deletions src/node_native_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/node_native_module.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 ids of builtin module source code into JS land as
Expand Down
6 changes: 4 additions & 2 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand Down
2 changes: 1 addition & 1 deletion test/message/error_exit.out
Original file line number Diff line number Diff line change
Expand Up @@ -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:*:*)
8 changes: 4 additions & 4 deletions test/message/eval_messages.out
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
Expand All @@ -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;
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/message/events_unhandled_error_common_trace.out
Original file line number Diff line number Diff line change
Expand Up @@ -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:*:*)
4 changes: 2 additions & 2 deletions test/message/events_unhandled_error_nexttick.out
Original file line number Diff line number Diff line change
Expand Up @@ -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:*:*)
4 changes: 2 additions & 2 deletions test/message/events_unhandled_error_sameline.out
Original file line number Diff line number Diff line change
Expand Up @@ -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:*:*)
2 changes: 1 addition & 1 deletion test/message/nexttick_throw.out
Original file line number Diff line number Diff line change
Expand Up @@ -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:*:*)