bootstrap: use a context snapshotted with primordials in workers

PR-URL: #42867 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
nodejs · May 10, 2022 · a20310d · a20310d
1 parent 2df1624
commit a20310d
Show file tree

Hide file tree

Showing 7 changed files with 111 additions and 73 deletions.
diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc
@@ -183,7 +183,7 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code) {
                               EnvironmentFlags::kDefaultFlags,
                               {}));
     context = Context::FromSnapshot(isolate_,
-                                    kNodeContextIndex,
+                                    SnapshotBuilder::kNodeMainContextIndex,
                                     {DeserializeNodeInternalFields, env.get()})
                   .ToLocalChecked();
 

diff --git a/src/node_main_instance.h b/src/node_main_instance.h
@@ -65,7 +65,6 @@ class NodeMainInstance {
   DeleteFnPtr<Environment, FreeEnvironment> CreateMainEnvironment(
       int* exit_code);
 
-  static const size_t kNodeContextIndex = 0;
   NodeMainInstance(const NodeMainInstance&) = delete;
   NodeMainInstance& operator=(const NodeMainInstance&) = delete;
   NodeMainInstance(NodeMainInstance&&) = delete;

diff --git a/src/node_snapshot_builder.h b/src/node_snapshot_builder.h
@@ -29,6 +29,9 @@ class SnapshotBuilder {
   static void InitializeIsolateParams(const SnapshotData* data,
                                       v8::Isolate::CreateParams* params);
 
+  static const size_t kNodeBaseContextIndex = 0;
+  static const size_t kNodeMainContextIndex = kNodeBaseContextIndex + 1;
+
  private:
   // Used to synchronize access to the snapshot data
   static Mutex snapshot_data_mutex_;

diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc
@@ -129,84 +129,101 @@ void SnapshotBuilder::Generate(SnapshotData* out,
                                    per_process::v8_platform.Platform(),
                                    args,
                                    exec_args);
+      out->isolate_data_indices =
+          main_instance->isolate_data()->Serialize(&creator);
 
       HandleScope scope(isolate);
+
+      // The default context with only things created by V8.
       creator.SetDefaultContext(Context::New(isolate));
-      out->isolate_data_indices =
-          main_instance->isolate_data()->Serialize(&creator);
 
-      // Run the per-context scripts
-      Local<Context> context;
-      {
+      auto CreateBaseContext = [&]() {
         TryCatch bootstrapCatch(isolate);
-        context = NewContext(isolate);
+        // Run the per-context scripts.
+        Local<Context> base_context = NewContext(isolate);
         if (bootstrapCatch.HasCaught()) {
-          PrintCaughtException(isolate, context, bootstrapCatch);
+          PrintCaughtException(isolate, base_context, bootstrapCatch);
           abort();
         }
+        return base_context;
+      };
+
+      // The Node.js-specific context with primodials, can be used by workers
+      // TODO(joyeecheung): investigate if this can be used by vm contexts
+      // without breaking compatibility.
+      {
+        size_t index = creator.AddContext(CreateBaseContext());
+        CHECK_EQ(index, SnapshotBuilder::kNodeBaseContextIndex);
       }
-      Context::Scope context_scope(context);
-
-      // Create the environment
-      env = new Environment(main_instance->isolate_data(),
-                            context,
-                            args,
-                            exec_args,
-                            nullptr,
-                            node::EnvironmentFlags::kDefaultFlags,
-                            {});
-
-      // Run scripts in lib/internal/bootstrap/
+
+      // The main instance context.
       {
+        Local<Context> main_context = CreateBaseContext();
+        Context::Scope context_scope(main_context);
         TryCatch bootstrapCatch(isolate);
+
+        // Create the environment.
+        env = new Environment(main_instance->isolate_data(),
+                              main_context,
+                              args,
+                              exec_args,
+                              nullptr,
+                              node::EnvironmentFlags::kDefaultFlags,
+                              {});
+
+        // Run scripts in lib/internal/bootstrap/
         MaybeLocal<Value> result = env->RunBootstrapping();
         if (bootstrapCatch.HasCaught()) {
-          PrintCaughtException(isolate, context, bootstrapCatch);
+          // TODO(joyeecheung): fail by exiting with a non-zero exit code.
+          PrintCaughtException(isolate, main_context, bootstrapCatch);
+          abort();
         }
         result.ToLocalChecked();
-      }
-
-      // If --build-snapshot is true, lib/internal/main/mksnapshot.js would be
-      // loaded via LoadEnvironment() to execute process.argv[1] as the entry
-      // point (we currently only support this kind of entry point, but we
-      // could also explore snapshotting other kinds of execution modes
-      // in the future).
-      if (per_process::cli_options->build_snapshot) {
+        // If --build-snapshot is true, lib/internal/main/mksnapshot.js would be
+        // loaded via LoadEnvironment() to execute process.argv[1] as the entry
+        // point (we currently only support this kind of entry point, but we
+        // could also explore snapshotting other kinds of execution modes
+        // in the future).
+        if (per_process::cli_options->build_snapshot) {
 #if HAVE_INSPECTOR
-        env->InitializeInspector({});
+          env->InitializeInspector({});
 #endif
-        TryCatch bootstrapCatch(isolate);
-        // TODO(joyeecheung): we could use the result for something special,
-        // like setting up initializers that should be invoked at snapshot
-        // dehydration.
-        MaybeLocal<Value> result =
-            LoadEnvironment(env, StartExecutionCallback{});
-        if (bootstrapCatch.HasCaught()) {
-          PrintCaughtException(isolate, context, bootstrapCatch);
+          // TODO(joyeecheung): we could use the result for something special,
+          // like setting up initializers that should be invoked at snapshot
+          // dehydration.
+          MaybeLocal<Value> result =
+              LoadEnvironment(env, StartExecutionCallback{});
+          if (bootstrapCatch.HasCaught()) {
+            // TODO(joyeecheung): fail by exiting with a non-zero exit code.
+            PrintCaughtException(isolate, main_context, bootstrapCatch);
+            abort();
+          }
+          result.ToLocalChecked();
+          // FIXME(joyeecheung): right now running the loop in the snapshot
+          // builder seems to introduces inconsistencies in JS land that need to
+          // be synchronized again after snapshot restoration.
+          int exit_code = SpinEventLoop(env).FromMaybe(1);
+          CHECK_EQ(exit_code, 0);
+          if (bootstrapCatch.HasCaught()) {
+            // TODO(joyeecheung): fail by exiting with a non-zero exit code.
+            PrintCaughtException(isolate, main_context, bootstrapCatch);
+            abort();
+          }
         }
-        result.ToLocalChecked();
-        // FIXME(joyeecheung): right now running the loop in the snapshot
-        // builder seems to introduces inconsistencies in JS land that need to
-        // be synchronized again after snapshot restoration.
-        int exit_code = SpinEventLoop(env).FromMaybe(1);
-        CHECK_EQ(exit_code, 0);
-        if (bootstrapCatch.HasCaught()) {
-          PrintCaughtException(isolate, context, bootstrapCatch);
-          abort();
+
+        if (per_process::enabled_debug_list.enabled(
+                DebugCategory::MKSNAPSHOT)) {
+          env->PrintAllBaseObjects();
+          printf("Environment = %p\n", env);
         }
-      }
 
-      if (per_process::enabled_debug_list.enabled(DebugCategory::MKSNAPSHOT)) {
-        env->PrintAllBaseObjects();
-        printf("Environment = %p\n", env);
+        // Serialize the native states
+        out->env_info = env->Serialize(&creator);
+        // Serialize the context
+        size_t index = creator.AddContext(
+            main_context, {SerializeNodeContextInternalFields, env});
+        CHECK_EQ(index, SnapshotBuilder::kNodeMainContextIndex);
       }
-
-      // Serialize the native states
-      out->env_info = env->Serialize(&creator);
-      // Serialize the context
-      size_t index = creator.AddContext(
-          context, {SerializeNodeContextInternalFields, env});
-      CHECK_EQ(index, NodeMainInstance::kNodeContextIndex);
     }
 
     // Must be out of HandleScope

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -50,13 +50,15 @@ Worker::Worker(Environment* env,
                const std::string& url,
                std::shared_ptr<PerIsolateOptions> per_isolate_opts,
                std::vector<std::string>&& exec_argv,
-               std::shared_ptr<KVStore> env_vars)
+               std::shared_ptr<KVStore> env_vars,
+               const SnapshotData* snapshot_data)
     : AsyncWrap(env, wrap, AsyncWrap::PROVIDER_WORKER),
       per_isolate_opts_(per_isolate_opts),
       exec_argv_(exec_argv),
       platform_(env->isolate_data()->platform()),
       thread_id_(AllocateEnvironmentThreadId()),
-      env_vars_(env_vars) {
+      env_vars_(env_vars),
+      snapshot_data_(snapshot_data) {
   Debug(this, "Creating new worker instance with thread id %llu",
         thread_id_.id);
 
@@ -147,12 +149,8 @@ class WorkerThreadData {
     SetIsolateCreateParamsForNode(&params);
     params.array_buffer_allocator_shared = allocator;
 
-    bool use_node_snapshot = per_process::cli_options->node_snapshot;
-    const SnapshotData* snapshot_data =
-        use_node_snapshot ? SnapshotBuilder::GetEmbeddedSnapshotData()
-                          : nullptr;
-    if (snapshot_data != nullptr) {
-      SnapshotBuilder::InitializeIsolateParams(snapshot_data, &params);
+    if (w->snapshot_data() != nullptr) {
+      SnapshotBuilder::InitializeIsolateParams(w->snapshot_data(), &params);
     }
     w->UpdateResourceConstraints(&params.constraints);
 
@@ -239,7 +237,7 @@ class WorkerThreadData {
   uv_loop_t loop_;
   bool loop_init_failed_ = true;
   DeleteFnPtr<IsolateData, FreeIsolateData> isolate_data_;
-
+  const SnapshotData* snapshot_data_ = nullptr;
   friend class Worker;
 };
 
@@ -302,7 +300,17 @@ void Worker::Run() {
         // resource constraints, we need something in place to handle it,
         // though.
         TryCatch try_catch(isolate_);
-        context = NewContext(isolate_);
+        if (snapshot_data_ != nullptr) {
+          context = Context::FromSnapshot(
+                        isolate_, SnapshotBuilder::kNodeBaseContextIndex)
+                        .ToLocalChecked();
+          if (!context.IsEmpty() &&
+              !InitializeContextRuntime(context).IsJust()) {
+            context = Local<Context>();
+          }
+        } else {
+          context = NewContext(isolate_);
+        }
         if (context.IsEmpty()) {
           Exit(1, "ERR_WORKER_INIT_FAILED", "Failed to create new Context");
           return;
@@ -560,12 +568,17 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
     exec_argv_out = env->exec_argv();
   }
 
+  bool use_node_snapshot = per_process::cli_options->node_snapshot;
+  const SnapshotData* snapshot_data =
+      use_node_snapshot ? SnapshotBuilder::GetEmbeddedSnapshotData() : nullptr;
+
   Worker* worker = new Worker(env,
                               args.This(),
                               url,
                               per_isolate_opts,
                               std::move(exec_argv_out),
-                              env_vars);
+                              env_vars,
+                              snapshot_data);
 
   CHECK(args[3]->IsFloat64Array());
   Local<Float64Array> limit_info = args[3].As<Float64Array>();

diff --git a/src/node_worker.h b/src/node_worker.h
@@ -9,6 +9,8 @@
 #include "uv.h"
 
 namespace node {
+
+struct SnapshotData;
 namespace worker {
 
 class WorkerThreadData;
@@ -29,7 +31,8 @@ class Worker : public AsyncWrap {
          const std::string& url,
          std::shared_ptr<PerIsolateOptions> per_isolate_opts,
          std::vector<std::string>&& exec_argv,
-         std::shared_ptr<KVStore> env_vars);
+         std::shared_ptr<KVStore> env_vars,
+         const SnapshotData* snapshot_data);
   ~Worker() override;
 
   // Run the worker. This is only called from the worker thread.
@@ -54,6 +57,7 @@ class Worker : public AsyncWrap {
   bool IsNotIndicativeOfMemoryLeakAtExit() const override;
 
   bool is_stopped() const;
+  const SnapshotData* snapshot_data() const { return snapshot_data_; }
 
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void CloneParentEnvVars(
@@ -126,6 +130,7 @@ class Worker : public AsyncWrap {
   // destroyed alongwith the worker thread.
   Environment* env_ = nullptr;
 
+  const SnapshotData* snapshot_data_ = nullptr;
   friend class WorkerThreadData;
 };
 

diff --git a/test/parallel/test-worker-stack-overflow-stack-size.js b/test/parallel/test-worker-stack-overflow-stack-size.js
@@ -23,7 +23,6 @@ async function runWorker(options = {}) {
   });
 
   const [ error ] = await once(worker, 'error');
-
   if (!options.skipErrorCheck) {
     common.expectsError({
       constructor: RangeError,
@@ -56,7 +55,9 @@ async function runWorker(options = {}) {
   }
 
   // Test that various low stack sizes result in an 'error' event.
-  for (const stackSizeMb of [ 0.001, 0.01, 0.1, 0.2, 0.3, 0.5 ]) {
+  // Currently the stack size needs to be at least 0.3MB for the worker to be
+  // bootstrapped properly.
+  for (const stackSizeMb of [ 0.3, 0.5, 1 ]) {
     await runWorker({ resourceLimits: { stackSizeMb }, skipErrorCheck: true });
   }
 })().then(common.mustCall());