Skip to content

src: split LoadEnvironment() at startExecution() #25320

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.

Sign up for GitHub

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

Closed
addaleax wants to merge 2 commits into from
Closed

src: split LoadEnvironment() at startExecution() #25320

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1351c49
src: split `LoadEnvironment()` at `startExecution()`
addaleax Jan 2, 2019
eb913cd
fixup! src: split `LoadEnvironment()` at `startExecution()`
addaleax Jan 3, 2019
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
4 changes: 2 additions & 2 deletions lib/internal/bootstrap/node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ function startup() {
} = perf.constants;
perf.markMilestone(NODE_PERFORMANCE_MILESTONE_BOOTSTRAP_COMPLETE);

startExecution();
return startExecution;
}

// There are various modes that Node can run in. The most common two
Expand Down Expand Up @@ -728,4 +728,4 @@ function checkScriptSyntax(source, filename) {
new vm.Script(source, { displayErrors: true, filename });
}

startup();
return startup();
8 changes: 8 additions & 0 deletions src/env-inl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,14 @@ inline void Environment::set_can_call_into_js(bool can_call_into_js) {
can_call_into_js_ = can_call_into_js;
}

inline bool Environment::has_run_bootstrapping_code() const {
return has_run_bootstrapping_code_;
}

inline void Environment::set_has_run_bootstrapping_code(bool value) {
has_run_bootstrapping_code_ = value;
}

inline bool Environment::is_main_thread() const {
return thread_id_ == 0;
}
Expand Down
5 changes: 5 additions & 0 deletions src/env.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ constexpr size_t kFsStatsBufferLength = kFsStatsFieldsNumber * 2;
V(script_data_constructor_function, v8::Function) \
V(secure_context_constructor_template, v8::FunctionTemplate) \
V(shutdown_wrap_template, v8::ObjectTemplate) \
V(start_execution_function, v8::Function) \
V(tcp_constructor_template, v8::FunctionTemplate) \
V(tick_callback_function, v8::Function) \
V(timers_callback_function, v8::Function) \
Expand Down Expand Up @@ -755,6 +756,9 @@ class Environment {
inline bool can_call_into_js() const;
inline void set_can_call_into_js(bool can_call_into_js);

inline bool has_run_bootstrapping_code() const;
inline void set_has_run_bootstrapping_code(bool has_run_bootstrapping_code);

inline bool is_main_thread() const;
inline uint64_t thread_id() const;
inline void set_thread_id(uint64_t id);
Expand Down Expand Up @@ -980,6 +984,7 @@ class Environment {
std::unique_ptr<performance::performance_state> performance_state_;
std::unordered_map<std::string, uint64_t> performance_marks_;

bool has_run_bootstrapping_code_ = false;
bool can_call_into_js_ = true;
uint64_t thread_id_ = 0;
std::unordered_set<worker::Worker*> sub_worker_contexts_;
Expand Down
25 changes: 23 additions & 2 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,14 @@ static MaybeLocal<Value> ExecuteBootstrapper(
}

void LoadEnvironment(Environment* env) {
RunBootstrapping(env);
StartExecution(env);
}

void RunBootstrapping(Environment* env) {
CHECK(!env->has_run_bootstrapping_code());
env->set_has_run_bootstrapping_code(true);

HandleScope handle_scope(env->isolate());
Isolate* isolate = env->isolate();
Local<Context> context = env->context();
Expand Down Expand Up @@ -1146,11 +1154,24 @@ void LoadEnvironment(Environment* env) {
loader_exports.ToLocalChecked(),
Boolean::New(isolate, env->is_main_thread())};

if (ExecuteBootstrapper(
Local<Value> start_execution;
if (!ExecuteBootstrapper(
env, "internal/bootstrap/node", &node_params, &node_args)
.IsEmpty()) {
.ToLocal(&start_execution)) {
return;
}

if (start_execution->IsFunction())
env->set_start_execution_function(start_execution.As<Function>());
}

void StartExecution(Environment* env) {
HandleScope handle_scope(env->isolate());
Local<Function> start_execution = env->start_execution_function();
if (start_execution.IsEmpty()) return;
start_execution->Call(
env->context(), Undefined(env->isolate()), 0, nullptr);
Comment thread
addaleax marked this conversation as resolved.
Outdated
env->set_start_execution_function(Local<Function>());
Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis Jan 3, 2019

Choose a reason for hiding this comment

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

Maybe do this before the call to, er, Call() as an extra guard against recursive invocation?

Copy link
Copy Markdown
Member Author

@addaleax addaleax Jan 3, 2019

Choose a reason for hiding this comment

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

Done, but it requires some extra magic because of the way the env->...() getters work…

}

static void StartInspector(Environment* env, const char* path) {
Expand Down
3 changes: 3 additions & 0 deletions src/node_internals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,9 @@ bool SafeGetenv(const char* key, std::string* text);

void DefineZlibConstants(v8::Local<v8::Object> target);

void RunBootstrapping(Environment* env);
void StartExecution(Environment* env);

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
1 change: 0 additions & 1 deletion test/message/assert_throws_stack.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,3 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly deep-equal:
at *
at *
at *
at *
1 change: 0 additions & 1 deletion test/message/error_exit.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,3 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at executeUserCode (internal/bootstrap/node.js:*:*)
at startExecution (internal/bootstrap/node.js:*:*)
at startup (internal/bootstrap/node.js:*:*)
8 changes: 0 additions & 8 deletions test/message/eval_messages.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ SyntaxError: Strict mode code may not include a with statement
at evalScript (internal/process/execution.js:*:*)
at executeUserCode (internal/bootstrap/node.js:*:*)
at startExecution (internal/bootstrap/node.js:*:*)
at startup (internal/bootstrap/node.js:*:*)
at internal/bootstrap/node.js:*:*
42
42
[eval]:1
Expand All @@ -28,8 +26,6 @@ Error: hello
at evalScript (internal/process/execution.js:*:*)
at executeUserCode (internal/bootstrap/node.js:*:*)
at startExecution (internal/bootstrap/node.js:*:*)
at startup (internal/bootstrap/node.js:*:*)
at internal/bootstrap/node.js:*:*

[eval]:1
throw new Error("hello")
Expand All @@ -44,8 +40,6 @@ Error: hello
at evalScript (internal/process/execution.js:*:*)
at executeUserCode (internal/bootstrap/node.js:*:*)
at startExecution (internal/bootstrap/node.js:*:*)
at startup (internal/bootstrap/node.js:*:*)
at internal/bootstrap/node.js:*:*
100
[eval]:1
var x = 100; y = x;
Expand All @@ -60,8 +54,6 @@ ReferenceError: y is not defined
at evalScript (internal/process/execution.js:*:*)
at executeUserCode (internal/bootstrap/node.js:*:*)
at startExecution (internal/bootstrap/node.js:*:*)
at startup (internal/bootstrap/node.js:*:*)
at internal/bootstrap/node.js:*:*

[eval]:1
var ______________________________________________; throw 10
Expand Down
3 changes: 0 additions & 3 deletions test/message/events_unhandled_error_nexttick.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@ Error
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at executeUserCode (internal/bootstrap/node.js:*:*)
at startExecution (internal/bootstrap/node.js:*:*)
at startup (internal/bootstrap/node.js:*:*)
Emitted 'error' event at:
at process.nextTick (*events_unhandled_error_nexttick.js:*:*)
at internalTickCallback (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 startup (internal/bootstrap/node.js:*:*)
at internal/bootstrap/node.js:*:*
3 changes: 1 addition & 2 deletions test/message/events_unhandled_error_sameline.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ Error
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at executeUserCode (internal/bootstrap/node.js:*:*)
at startExecution (internal/bootstrap/node.js:*:*)
at startup (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 startup (internal/bootstrap/node.js:*:*)
at startExecution (internal/bootstrap/node.js:*:*)
2 changes: 0 additions & 2 deletions test/message/nexttick_throw.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,3 @@ ReferenceError: undefined_reference_error_maker is not defined
at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
at executeUserCode (internal/bootstrap/node.js:*:*)
at startExecution (internal/bootstrap/node.js:*:*)
at startup (internal/bootstrap/node.js:*:*)
at internal/bootstrap/node.js:*:*
6 changes: 0 additions & 6 deletions test/message/unhandled_promise_trace_warnings.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
at *
at *
at *
at *
at *
at *
(node:*) Error: This was rejected
at * (*test*message*unhandled_promise_trace_warnings.js:*)
at *
Expand All @@ -28,7 +25,6 @@
at *
at *
at *
at *
(node:*) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
at *
at *
Expand All @@ -38,8 +34,6 @@
at *
at *
at *
at *
at *
(node:*) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 1)
at handledRejection (internal/process/promises.js:*)
at promiseRejectHandler (internal/process/promises.js:*)
Expand Down
5 changes: 0 additions & 5 deletions test/message/util_inspect_error.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
at *
at *
at *
at *
nested:
{ err:
Error: foo
Expand All @@ -23,7 +22,6 @@
at *
at *
at *
at *
at * } }
{
err: Error: foo
Expand All @@ -36,7 +34,6 @@
at *
at *
at *
at *
at *,
nested: {
err: Error: foo
Expand All @@ -50,7 +47,6 @@
at *
at *
at *
at *
}
}
{ Error: foo
Expand All @@ -64,5 +60,4 @@ bar
at *
at *
at *
at *
foo: 'bar' }