From 4d06ef468efdb131c24fa6df385d33206af33a8c Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 20 Mar 2019 00:12:23 +0800 Subject: [PATCH] process: run RunBootstrapping in CreateEnvironment Also creates `CreateMainEnvironment` to encapsulate the code creating the main environment from the provided Isolate data and arguments. PR-URL: https://github.com/nodejs/node/pull/26788 Reviewed-By: Anna Henningsen Reviewed-By: Refael Ackermann Signed-off-by: Beth Griggs --- lib/internal/bootstrap/cache.js | 1 + lib/internal/bootstrap/environment.js | 13 +++ node.gyp | 1 + src/api/environment.cc | 17 +++ src/node.cc | 147 +++++++++++++++----------- src/node.h | 2 + src/node_internals.h | 7 +- test/cctest/node_test_fixture.h | 2 - test/cctest/test_environment.cc | 18 ++++ 9 files changed, 141 insertions(+), 67 deletions(-) create mode 100644 lib/internal/bootstrap/environment.js diff --git a/lib/internal/bootstrap/cache.js b/lib/internal/bootstrap/cache.js index a68060c19643df..60450da805d22c 100644 --- a/lib/internal/bootstrap/cache.js +++ b/lib/internal/bootstrap/cache.js @@ -20,6 +20,7 @@ const cannotBeRequired = [ 'internal/test/binding', + 'internal/bootstrap/environment', 'internal/bootstrap/primordials', 'internal/bootstrap/loaders', 'internal/bootstrap/node', diff --git a/lib/internal/bootstrap/environment.js b/lib/internal/bootstrap/environment.js new file mode 100644 index 00000000000000..79a67dae378202 --- /dev/null +++ b/lib/internal/bootstrap/environment.js @@ -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(); diff --git a/node.gyp b/node.gyp index 1a0738008796d3..f6d8d8c6cbe734 100644 --- a/node.gyp +++ b/node.gyp @@ -29,6 +29,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', diff --git a/src/api/environment.cc b/src/api/environment.cc index c6da872ae95fc0..735630e72b6b9c 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -243,6 +243,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> parameters = { + env->require_string(), + FIXED_ONE_BYTE_STRING(env->isolate(), "markBootstrapComplete")}; + std::vector> arguments = { + env->native_module_require(), + env->NewFunctionTemplate(MarkBootstrapComplete) + ->GetFunction(env->context()) + .ToLocalChecked()}; + if (ExecuteBootstrapper( + env, "internal/bootstrap/environment", ¶meters, &arguments) + .IsEmpty()) { + return nullptr; + } return env; } diff --git a/src/node.cc b/src/node.cc index ae4465aa2ae23e..f53cb98ee7681e 100644 --- a/src/node.cc +++ b/src/node.cc @@ -200,11 +200,10 @@ void SignalExit(int signo) { raise(signo); } -static MaybeLocal ExecuteBootstrapper( - Environment* env, - const char* id, - std::vector>* parameters, - std::vector>* arguments) { +MaybeLocal ExecuteBootstrapper(Environment* env, + const char* id, + std::vector>* parameters, + std::vector>* arguments) { EscapableHandleScope scope(env->isolate()); MaybeLocal maybe_fn = per_process::native_module_loader.LookupAndCompile( @@ -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)); } @@ -780,90 +777,117 @@ void RunBeforeExit(Environment* env) { EmitBeforeExit(env); } -inline int StartNodeWithIsolate(Isolate* isolate, - IsolateData* isolate_data, - const std::vector& args, - const std::vector& 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 CreateMainEnvironment( + IsolateData* isolate_data, + const std::vector& args, + const std::vector& 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) { + isolate->GetHeapProfiler()->StartTrackingHeapObjects(true); + } + Local context = NewContext(isolate); Context::Scope context_scope(context); - int exit_code = 0; - Environment env( + + std::unique_ptr env = std::make_unique( isolate_data, context, static_cast(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& args, + const std::vector& exec_args) { + int exit_code = 0; + std::unique_ptr 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 @@ -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); } diff --git a/src/node.h b/src/node.h index 3717ab08490616..31b7eb6d6fa8cf 100644 --- a/src/node.h +++ b/src/node.h @@ -299,6 +299,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 context, int argc, diff --git a/src/node_internals.h b/src/node_internals.h index 687c7115f57a0c..44b68b4ccdb907 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -302,7 +302,12 @@ v8::MaybeLocal RunBootstrapping(Environment* env); v8::MaybeLocal StartExecution(Environment* env, const char* main_script_id); v8::MaybeLocal GetPerContextExports(v8::Local context); - +v8::MaybeLocal ExecuteBootstrapper( + Environment* env, + const char* id, + std::vector>* parameters, + std::vector>* arguments); +void MarkBootstrapComplete(const v8::FunctionCallbackInfo& args); namespace profiler { void StartCoverageCollection(Environment* env); } diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h index 0359ec3c76a939..a6b704d13fe6b1 100644 --- a/test/cctest/node_test_fixture.h +++ b/test/cctest/node_test_fixture.h @@ -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() { diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index aba4b719477e03..1ec4ae21008a55 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -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 context = isolate_->GetCurrentContext(); + + const char* run_script = "process.argv0"; + v8::Local script = v8::Script::Compile( + context, + v8::String::NewFromOneByte(isolate_, + reinterpret_cast(run_script), + v8::NewStringType::kNormal).ToLocalChecked()) + .ToLocalChecked(); + v8::Local result = script->Run(context).ToLocalChecked(); + CHECK(result->IsString()); +} + TEST_F(EnvironmentTest, AtExitWithEnvironment) { const v8::HandleScope handle_scope(isolate_); const Argv argv;