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: run RunBootstrapping in CreateEnvironment #26788

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
1 change: 1 addition & 0 deletions lib/internal/bootstrap/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const cannotBeRequired = [

'internal/test/binding',

'internal/bootstrap/environment',
'internal/bootstrap/primordials',
'internal/bootstrap/loaders',
'internal/bootstrap/node',
Expand Down
13 changes: 13 additions & 0 deletions lib/internal/bootstrap/environment.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
'use strict';

// This runs necessary preparations to prepare a complete Node.js context
// that depends on run time states.
// It is currently only intended for preparing contexts for embedders.

/* global markBootstrapComplete */
const {
prepareMainThreadExecution
} = require('internal/bootstrap/pre_execution');

prepareMainThreadExecution();
markBootstrapComplete();
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
'library_files': [
'lib/internal/bootstrap/primordials.js',
'lib/internal/bootstrap/cache.js',
'lib/internal/bootstrap/environment.js',
'lib/internal/bootstrap/loaders.js',
'lib/internal/bootstrap/node.js',
'lib/internal/bootstrap/pre_execution.js',
Expand Down
17 changes: 17 additions & 0 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,23 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
Environment::kOwnsInspector));
env->InitializeLibuv(per_process::v8_is_profiling);
env->ProcessCliArgs(args, exec_args);
if (RunBootstrapping(env).IsEmpty()) {
return nullptr;
}

std::vector<Local<String>> parameters = {
Copy link
Member Author

@joyeecheung joyeecheung Mar 22, 2019

Choose a reason for hiding this comment

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

Since we are splitting RunBootstrapping and StartExecution, this requires the user not to pass any arguments that result in an async operation done in pre-execution at the moment. This is not ideal, but we may be able to split what's done in environment.js in the future to make sure nothing async is done during that...then the bootstrap will be divided into three phases:

  1. Up to node.js: no async operation is allowed, no access to run time states
  2. environment.js: no async operation is allowed, but access to run time states is allowed (warnings emitted here must be done synchronously)
  3. pre_execution.js: both are allowed, but the embedder would have to call this themselves somehow and make sure it plays with how they run the event loops.

env->require_string(),
FIXED_ONE_BYTE_STRING(env->isolate(), "markBootstrapComplete")};
std::vector<Local<Value>> arguments = {
env->native_module_require(),
env->NewFunctionTemplate(MarkBootstrapComplete)
->GetFunction(env->context())
.ToLocalChecked()};
if (ExecuteBootstrapper(
env, "internal/bootstrap/environment", &parameters, &arguments)
.IsEmpty()) {
return nullptr;
}
return env;
}

Expand Down
147 changes: 83 additions & 64 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,10 @@ void SignalExit(int signo) {
raise(signo);
}

static MaybeLocal<Value> ExecuteBootstrapper(
Environment* env,
const char* id,
std::vector<Local<String>>* parameters,
std::vector<Local<Value>>* arguments) {
MaybeLocal<Value> ExecuteBootstrapper(Environment* env,
const char* id,
std::vector<Local<String>>* parameters,
std::vector<Local<Value>>* arguments) {
EscapableHandleScope scope(env->isolate());
MaybeLocal<Function> maybe_fn =
per_process::native_module_loader.LookupAndCompile(
Expand Down Expand Up @@ -453,9 +452,7 @@ void LoadEnvironment(Environment* env) {
// StartMainThreadExecution() make sense for embedders. Pick the
// useful ones out, and allow embedders to customize the entry
// point more directly without using _third_party_main.js
if (!RunBootstrapping(env).IsEmpty()) {
USE(StartMainThreadExecution(env));
}
USE(StartMainThreadExecution(env));
}


Expand Down Expand Up @@ -780,90 +777,117 @@ void RunBeforeExit(Environment* env) {
EmitBeforeExit(env);
}

inline int StartNodeWithIsolate(Isolate* isolate,
IsolateData* isolate_data,
const std::vector<std::string>& args,
const std::vector<std::string>& exec_args) {
// TODO(joyeecheung): align this with the CreateEnvironment exposed in node.h
// and the environment creation routine in workers somehow.
inline std::unique_ptr<Environment> CreateMainEnvironment(
IsolateData* isolate_data,
const std::vector<std::string>& args,
const std::vector<std::string>& exec_args,
int* exit_code) {
Isolate* isolate = isolate_data->isolate();
HandleScope handle_scope(isolate);

// TODO(addaleax): This should load a real per-Isolate option, currently
// this is still effectively per-process.
if (isolate_data->options()->track_heap_objects) {
Copy link
Member Author

@joyeecheung joyeecheung Mar 19, 2019

Choose a reason for hiding this comment

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

My thinking was when we implement snapshot, this could be where the context is deserialized from the snapshot (as opposed to using NewContext). Note that to deserialize the snapshot properly we need to reinstall per-isolate callbacks, including the ones installed in SetIsolateUpForNode, so we may need to stop using NewIsolate at that point. We also need to restore inspector states after deserialization as those are discarded when the snapshot is generated, so it makes sense to me to encapsulate all this together.

isolate->GetHeapProfiler()->StartTrackingHeapObjects(true);
}

Local<Context> context = NewContext(isolate);
Context::Scope context_scope(context);
int exit_code = 0;
Environment env(

std::unique_ptr<Environment> env = std::make_unique<Environment>(
isolate_data,
context,
static_cast<Environment::Flags>(Environment::kIsMainThread |
Environment::kOwnsProcessState |
Environment::kOwnsInspector));
env.InitializeLibuv(per_process::v8_is_profiling);
env.ProcessCliArgs(args, exec_args);
env->InitializeLibuv(per_process::v8_is_profiling);
env->ProcessCliArgs(args, exec_args);

#if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
CHECK(!env.inspector_agent()->IsListening());
CHECK(!env->inspector_agent()->IsListening());
// Inspector agent can't fail to start, but if it was configured to listen
// right away on the websocket port and fails to bind/etc, this will return
// false.
env.inspector_agent()->Start(args.size() > 1 ? args[1].c_str() : "",
env.options()->debug_options(),
env.inspector_host_port(),
true);
if (env.options()->debug_options().inspector_enabled &&
!env.inspector_agent()->IsListening()) {
exit_code = 12; // Signal internal error.
goto exit;
env->inspector_agent()->Start(args.size() > 1 ? args[1].c_str() : "",
env->options()->debug_options(),
env->inspector_host_port(),
true);
if (env->options()->debug_options().inspector_enabled &&
!env->inspector_agent()->IsListening()) {
*exit_code = 12; // Signal internal error.
return env;
}
#else
// inspector_enabled can't be true if !HAVE_INSPECTOR or !NODE_USE_V8_PLATFORM
// - the option parser should not allow that.
CHECK(!env.options()->debug_options().inspector_enabled);
CHECK(!env->options()->debug_options().inspector_enabled);
#endif // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM

{
AsyncCallbackScope callback_scope(&env);
env.async_hooks()->push_async_ids(1, 0);
LoadEnvironment(&env);
env.async_hooks()->pop_async_id(1);
if (RunBootstrapping(env.get()).IsEmpty()) {
*exit_code = 1;
}

{
SealHandleScope seal(isolate);
bool more;
env.performance_state()->Mark(
node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_START);
do {
uv_run(env.event_loop(), UV_RUN_DEFAULT);
return env;
}

per_process::v8_platform.DrainVMTasks(isolate);
inline int StartNodeWithIsolate(Isolate* isolate,
IsolateData* isolate_data,
const std::vector<std::string>& args,
const std::vector<std::string>& exec_args) {
int exit_code = 0;
std::unique_ptr<Environment> env =
CreateMainEnvironment(isolate_data, args, exec_args, &exit_code);
CHECK_NOT_NULL(env);
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());

if (exit_code == 0) {
{
AsyncCallbackScope callback_scope(env.get());
env->async_hooks()->push_async_ids(1, 0);
LoadEnvironment(env.get());
env->async_hooks()->pop_async_id(1);
}

more = uv_loop_alive(env.event_loop());
if (more && !env.is_stopping()) continue;
{
SealHandleScope seal(isolate);
bool more;
env->performance_state()->Mark(
node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_START);
do {
uv_run(env->event_loop(), UV_RUN_DEFAULT);

RunBeforeExit(&env);
per_process::v8_platform.DrainVMTasks(isolate);

// Emit `beforeExit` if the loop became alive either after emitting
// event, or after running some callbacks.
more = uv_loop_alive(env.event_loop());
} while (more == true && !env.is_stopping());
env.performance_state()->Mark(
node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_EXIT);
}
more = uv_loop_alive(env->event_loop());
if (more && !env->is_stopping()) continue;

env.set_trace_sync_io(false);
RunBeforeExit(env.get());

exit_code = EmitExit(&env);
// Emit `beforeExit` if the loop became alive either after emitting
// event, or after running some callbacks.
more = uv_loop_alive(env->event_loop());
} while (more == true && !env->is_stopping());
env->performance_state()->Mark(
node::performance::NODE_PERFORMANCE_MILESTONE_LOOP_EXIT);
}

WaitForInspectorDisconnect(&env);
env->set_trace_sync_io(false);
exit_code = EmitExit(env.get());
WaitForInspectorDisconnect(env.get());
}

#if HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
exit:
#endif
env.set_can_call_into_js(false);
env.stop_sub_worker_contexts();
env->set_can_call_into_js(false);
env->stop_sub_worker_contexts();
uv_tty_reset_mode();
env.RunCleanup();
RunAtExit(&env);
env->RunCleanup();
RunAtExit(env.get());

per_process::v8_platform.DrainVMTasks(isolate);
per_process::v8_platform.CancelVMTasks(isolate);

#if defined(LEAK_SANITIZER)
__lsan_do_leak_check();
#endif
Expand Down Expand Up @@ -891,11 +915,6 @@ inline int StartNodeWithLoopAndArgs(uv_loop_t* event_loop,
per_process::v8_platform.Platform(),
allocator.get()),
&FreeIsolateData);
// TODO(addaleax): This should load a real per-Isolate option, currently
// this is still effectively per-process.
if (isolate_data->options()->track_heap_objects) {
isolate->GetHeapProfiler()->StartTrackingHeapObjects(true);
}
exit_code =
StartNodeWithIsolate(isolate, isolate_data.get(), args, exec_args);
}
Expand Down
2 changes: 2 additions & 0 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ NODE_EXTERN void FreeIsolateData(IsolateData* isolate_data);

// TODO(addaleax): Add an official variant using STL containers, and move
// per-Environment options parsing here.
// Returns nullptr when the Environment cannot be created e.g. there are
// pending JavaScript exceptions.
NODE_EXTERN Environment* CreateEnvironment(IsolateData* isolate_data,
v8::Local<v8::Context> context,
int argc,
Expand Down
7 changes: 6 additions & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,12 @@ v8::MaybeLocal<v8::Value> RunBootstrapping(Environment* env);
v8::MaybeLocal<v8::Value> StartExecution(Environment* env,
const char* main_script_id);
v8::MaybeLocal<v8::Object> GetPerContextExports(v8::Local<v8::Context> context);

v8::MaybeLocal<v8::Value> ExecuteBootstrapper(
Environment* env,
const char* id,
std::vector<v8::Local<v8::String>>* parameters,
std::vector<v8::Local<v8::Value>>* arguments);
void MarkBootstrapComplete(const v8::FunctionCallbackInfo<v8::Value>& args);
namespace profiler {
void StartCoverageCollection(Environment* env);
}
Expand Down
2 changes: 0 additions & 2 deletions test/cctest/node_test_fixture.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,6 @@ class EnvironmentTestFixture : public NodeTestFixture {
1, *argv,
argv.nr_args(), *argv);
CHECK_NE(nullptr, environment_);
// TODO(addaleax): Make this a public API.
CHECK(!RunBootstrapping(environment_).IsEmpty());
}

~Env() {
Expand Down
18 changes: 18 additions & 0 deletions test/cctest/test_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,24 @@ class EnvironmentTest : public EnvironmentTestFixture {
}
};

TEST_F(EnvironmentTest, PreExeuctionPreparation) {
const v8::HandleScope handle_scope(isolate_);
const Argv argv;
Env env {handle_scope, argv};

v8::Local<v8::Context> context = isolate_->GetCurrentContext();

const char* run_script = "process.argv0";
v8::Local<v8::Script> script = v8::Script::Compile(
context,
v8::String::NewFromOneByte(isolate_,
reinterpret_cast<const uint8_t*>(run_script),
v8::NewStringType::kNormal).ToLocalChecked())
.ToLocalChecked();
v8::Local<v8::Value> result = script->Run(context).ToLocalChecked();
CHECK(result->IsString());
}

TEST_F(EnvironmentTest, AtExitWithEnvironment) {
const v8::HandleScope handle_scope(isolate_);
const Argv argv;
Expand Down