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: nodejs#26788 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Refael Ackermann <refack@gmail.com>
cjihrig · Apr 3, 2019 · 1944265 · 1944265
1 parent e02f511
commit 1944265
Show file tree

Hide file tree

Showing 9 changed files with 141 additions and 67 deletions.
diff --git 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
@@ -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
@@ -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',

diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -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 = {
+      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;
 }
 

diff --git a/src/node.cc b/src/node.cc
@@ -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(
@@ -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<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) {
+    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
@@ -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
@@ -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,

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -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);
 }

diff --git 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
@@ -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;