Skip to content

Commit

Permalink
src: split LoadEnvironment() at startExecution()
Browse files Browse the repository at this point in the history
This makes it easier to cater to embedders which wish to skip
the `startExecution()` part.

PR-URL: #25320
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
  • Loading branch information
addaleax committed Jan 6, 2019
1 parent 728505a commit f6a1d88
Show file tree
Hide file tree
Showing 13 changed files with 47 additions and 32 deletions.
4 changes: 2 additions & 2 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,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 @@ -729,4 +729,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
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
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
30 changes: 28 additions & 2 deletions src/node.cc
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,29 @@ 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());
// 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
// resetting the handle while we're accessing the object through the Local<>.
Local<Function> start_execution =
Local<Function>::New(env->isolate(), env->start_execution_function());
env->set_start_execution_function(Local<Function>());

if (start_execution.IsEmpty()) return;
USE(start_execution->Call(
env->context(), Undefined(env->isolate()), 0, nullptr));
}

static void StartInspector(Environment* env, const char* path) {
Expand Down
3 changes: 3 additions & 0 deletions src/node_internals.h
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
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
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
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
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
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
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
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
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' }

0 comments on commit f6a1d88

Please sign in to comment.