nodejs · joyeecheung · Apr 11, 2022 · Apr 11, 2022 · Apr 11, 2022 · Apr 18, 2022
diff --git a/node.gyp b/node.gyp
@@ -637,6 +637,7 @@
         'src/node_revert.h',
         'src/node_root_certs.h',
         'src/node_snapshotable.h',
+        'src/node_snapshot_builder.h',
         'src/node_sockaddr.h',
         'src/node_sockaddr-inl.h',
         'src/node_stat_watcher.h',

diff --git a/src/node.cc b/src/node.cc
@@ -25,8 +25,8 @@
 
 #include "debug_utils-inl.h"
 #include "env-inl.h"
-#include "memory_tracker-inl.h"
 #include "histogram-inl.h"
+#include "memory_tracker-inl.h"
 #include "node_binding.h"
 #include "node_errors.h"
 #include "node_internals.h"
@@ -38,6 +38,7 @@
 #include "node_process-inl.h"
 #include "node_report.h"
 #include "node_revert.h"
+#include "node_snapshot_builder.h"
 #include "node_v8_platform-inl.h"
 #include "node_version.h"
 
@@ -1171,7 +1172,7 @@ int Start(int argc, char** argv) {
     bool use_node_snapshot =
         per_process::cli_options->per_isolate->node_snapshot;
     const SnapshotData* snapshot_data =
-        use_node_snapshot ? NodeMainInstance::GetEmbeddedSnapshotData()
+        use_node_snapshot ? SnapshotBuilder::GetEmbeddedSnapshotData()
                           : nullptr;
     uv_loop_configure(uv_default_loop(), UV_METRICS_IDLE_TIME);
 

diff --git a/src/node_external_reference.cc b/src/node_external_reference.cc
@@ -7,9 +7,11 @@
 namespace node {
 
 const std::vector<intptr_t>& ExternalReferenceRegistry::external_references() {
-  CHECK(!is_finalized_);
-  external_references_.push_back(reinterpret_cast<intptr_t>(nullptr));
-  is_finalized_ = true;
+  if (!is_finalized_) {
+    external_references_.push_back(reinterpret_cast<intptr_t>(nullptr));
+    is_finalized_ = true;
+  }
+
   return external_references_;
 }
 

diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc
@@ -7,6 +7,7 @@
 #include "node_external_reference.h"
 #include "node_internals.h"
 #include "node_options-inl.h"
+#include "node_snapshot_builder.h"
 #include "node_snapshotable.h"
 #include "node_v8_platform-inl.h"
 #include "util-inl.h"
@@ -26,8 +27,6 @@ using v8::Isolate;
 using v8::Local;
 using v8::Locker;
 
-std::unique_ptr<ExternalReferenceRegistry> NodeMainInstance::registry_ =
-    nullptr;
 NodeMainInstance::NodeMainInstance(Isolate* isolate,
                                    uv_loop_t* event_loop,
                                    MultiIsolatePlatform* platform,
@@ -46,13 +45,6 @@ NodeMainInstance::NodeMainInstance(Isolate* isolate,
   SetIsolateMiscHandlers(isolate_, {});
 }
 
-const std::vector<intptr_t>& NodeMainInstance::CollectExternalReferences() {
-  // Cannot be called more than once.
-  CHECK_NULL(registry_);
-  registry_.reset(new ExternalReferenceRegistry());
-  return registry_->external_references();
-}
-
 std::unique_ptr<NodeMainInstance> NodeMainInstance::Create(
     Isolate* isolate,
     uv_loop_t* event_loop,
@@ -78,13 +70,8 @@ NodeMainInstance::NodeMainInstance(const SnapshotData* snapshot_data,
       snapshot_data_(snapshot_data) {
   isolate_params_->array_buffer_allocator = array_buffer_allocator_.get();
   if (snapshot_data != nullptr) {
-    // TODO(joyeecheung): collect external references and set it in
-    // params.external_references.
-    const std::vector<intptr_t>& external_references =
-        CollectExternalReferences();
-    isolate_params_->external_references = external_references.data();
-    isolate_params_->snapshot_blob =
-        const_cast<v8::StartupData*>(&(snapshot_data->blob));
+    SnapshotBuilder::InitializeIsolateParams(snapshot_data,
+                                             isolate_params_.get());
   }
 
   isolate_ = Isolate::Allocate();

diff --git a/src/node_main_instance.h b/src/node_main_instance.h
@@ -65,11 +65,6 @@ class NodeMainInstance {
   DeleteFnPtr<Environment, FreeEnvironment> CreateMainEnvironment(
       int* exit_code);
 
-  // If nullptr is returned, the binary is not built with embedded
-  // snapshot.
-  static const SnapshotData* GetEmbeddedSnapshotData();
-  static const std::vector<intptr_t>& CollectExternalReferences();
-
   static const size_t kNodeContextIndex = 0;
   NodeMainInstance(const NodeMainInstance&) = delete;
   NodeMainInstance& operator=(const NodeMainInstance&) = delete;

diff --git a/src/node_snapshot_builder.h b/src/node_snapshot_builder.h
@@ -0,0 +1,43 @@
+
+#ifndef SRC_NODE_SNAPSHOT_BUILDER_H_
+#define SRC_NODE_SNAPSHOT_BUILDER_H_
+
+#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#include <cstdint>
+#include "node_mutex.h"
+#include "v8.h"
+
+namespace node {
+
+class ExternalReferenceRegistry;
+struct SnapshotData;
+
+class SnapshotBuilder {
+ public:
+  static std::string Generate(const std::vector<std::string> args,
+                              const std::vector<std::string> exec_args);
+
+  // Generate the snapshot into out.
+  static void Generate(SnapshotData* out,
+                       const std::vector<std::string> args,
+                       const std::vector<std::string> exec_args);
+
+  // If nullptr is returned, the binary is not built with embedded
+  // snapshot.
+  static const SnapshotData* GetEmbeddedSnapshotData();
+  static void InitializeIsolateParams(const SnapshotData* data,
+                                      v8::Isolate::CreateParams* params);
+
+ private:
+  // Used to synchronize access to the snapshot data
+  static Mutex snapshot_data_mutex_;
+  static const std::vector<intptr_t>& CollectExternalReferences();
+
+  static std::unique_ptr<ExternalReferenceRegistry> registry_;
+};
+}  // namespace node
+
+#endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#endif  // SRC_NODE_SNAPSHOT_BUILDER_H_
diff --git a/src/node_snapshot_stub.cc b/src/node_snapshot_stub.cc
@@ -2,11 +2,11 @@
 // NODE_WANT_INTERNALS, so we define it here manually.
 #define NODE_WANT_INTERNALS 1
 
-#include "node_main_instance.h"
+#include "node_snapshot_builder.h"
 
 namespace node {
 
-const SnapshotData* NodeMainInstance::GetEmbeddedSnapshotData() {
+const SnapshotData* SnapshotBuilder::GetEmbeddedSnapshotData() {
   return nullptr;
 }
 

diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc
@@ -12,6 +12,7 @@
 #include "node_internals.h"
 #include "node_main_instance.h"
 #include "node_process.h"
+#include "node_snapshot_builder.h"
 #include "node_v8.h"
 #include "node_v8_platform-inl.h"
 
@@ -49,7 +50,7 @@ std::string FormatBlob(SnapshotData* data) {
 
   ss << R"(#include <cstddef>
 #include "env.h"
-#include "node_main_instance.h"
+#include "node_snapshot_builder.h"
 #include "v8.h"
 
 // This file is generated by tools/snapshot. Do not edit.
@@ -78,11 +79,12 @@ SnapshotData snapshot_data {
   // -- isolate_data_indices ends --
   // -- env_info begins --
 )" << data->env_info
-  << R"(
+     << R"(
   // -- env_info ends --
 };
 
-const SnapshotData* NodeMainInstance::GetEmbeddedSnapshotData() {
+const SnapshotData* SnapshotBuilder::GetEmbeddedSnapshotData() {
+  Mutex::ScopedLock lock(snapshot_data_mutex_);
   return &snapshot_data;
 }
 }  // namespace node
@@ -91,6 +93,19 @@ const SnapshotData* NodeMainInstance::GetEmbeddedSnapshotData() {
   return ss.str();
 }
 
+Mutex SnapshotBuilder::snapshot_data_mutex_;
+
+const std::vector<intptr_t>& SnapshotBuilder::CollectExternalReferences() {
+  static auto registry = std::make_unique<ExternalReferenceRegistry>();
+  return registry->external_references();
+}
+
+void SnapshotBuilder::InitializeIsolateParams(const SnapshotData* data,
+                                              Isolate::CreateParams* params) {
+  params->external_references = CollectExternalReferences().data();
+  params->snapshot_blob = const_cast<v8::StartupData*>(&(data->blob));
+}
+
 void SnapshotBuilder::Generate(SnapshotData* out,
                                const std::vector<std::string> args,
                                const std::vector<std::string> exec_args) {
@@ -104,7 +119,7 @@ void SnapshotBuilder::Generate(SnapshotData* out,
 
   {
     const std::vector<intptr_t>& external_references =
-        NodeMainInstance::CollectExternalReferences();
+        CollectExternalReferences();
     SnapshotCreator creator(isolate, external_references.data());
     Environment* env;
     {

diff --git a/src/node_snapshotable.h b/src/node_snapshotable.h
@@ -12,6 +12,7 @@ namespace node {
 class Environment;
 struct EnvSerializeInfo;
 struct SnapshotData;
+class ExternalReferenceRegistry;
 
 #define SERIALIZABLE_OBJECT_TYPES(V)                                           \
   V(fs_binding_data, fs::BindingData)                                          \
@@ -122,17 +123,6 @@ void SerializeBindingData(Environment* env,
                           EnvSerializeInfo* info);
 
 bool IsSnapshotableType(FastStringKey key);
-
-class SnapshotBuilder {
- public:
-  static std::string Generate(const std::vector<std::string> args,
-                              const std::vector<std::string> exec_args);
-
-  // Generate the snapshot into out.
-  static void Generate(SnapshotData* out,
-                       const std::vector<std::string> args,
-                       const std::vector<std::string> exec_args);
-};
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -7,6 +7,7 @@
 #include "node_buffer.h"
 #include "node_options-inl.h"
 #include "node_perf.h"
+#include "node_snapshot_builder.h"
 #include "util-inl.h"
 #include "async_wrap-inl.h"
 
@@ -146,6 +147,20 @@ class WorkerThreadData {
     SetIsolateCreateParamsForNode(&params);
     params.array_buffer_allocator_shared = allocator;
 
+    bool use_node_snapshot = true;
+    if (w_->per_isolate_opts_) {
+      use_node_snapshot = w_->per_isolate_opts_->node_snapshot;
+    } else {
+      // IsolateData is created after the Isolate is created so we'll
+      // inherit the option from the parent here.
+      use_node_snapshot = per_process::cli_options->per_isolate->node_snapshot;
+    }
+    const SnapshotData* snapshot_data =
+        use_node_snapshot ? SnapshotBuilder::GetEmbeddedSnapshotData()
+                          : nullptr;
+    if (snapshot_data != nullptr) {
+      SnapshotBuilder::InitializeIsolateParams(snapshot_data, &params);
+    }
     w->UpdateResourceConstraints(&params.constraints);
 
     Isolate* isolate = Isolate::Allocate();

diff --git a/test/parallel/test-worker-nearheaplimit-deadlock.js b/test/parallel/test-worker-nearheaplimit-deadlock.js
@@ -10,7 +10,11 @@ if (!process.env.HAS_STARTED_WORKER) {
     resourceLimits: {
       maxYoungGenerationSizeMb: 0,
       maxOldGenerationSizeMb: 0
-    }
+    },
+    // With node snapshot the OOM can occur during the deserialization of
+    // the context, so disable it since we want the OOM to occur during
+    // the creation of the message port.
+    execArgv: [ ...process.execArgv, '--no-node-snapshot']
   };
 
   const worker = new Worker(__filename, opts);

diff --git a/test/parallel/test-worker-resource-limits.js b/test/parallel/test-worker-resource-limits.js
@@ -35,10 +35,10 @@ if (!process.env.HAS_STARTED_WORKER) {
 assert.deepStrictEqual(resourceLimits, testResourceLimits);
 const array = [];
 while (true) {
-  // Leave 10% wiggle room here, and 20% on debug builds.
-  const wiggleRoom = common.buildType === 'Release' ? 1.1 : 1.2;
   const usedMB = v8.getHeapStatistics().used_heap_size / 1024 / 1024;
-  assert(usedMB < resourceLimits.maxOldGenerationSizeMb * wiggleRoom);
+  const maxReservedSize = resourceLimits.maxOldGenerationSizeMb +
+                          resourceLimits.maxYoungGenerationSizeMb;
+  assert(usedMB < maxReservedSize);
 
   let seenSpaces = 0;
   for (const { space_name, space_size } of v8.getHeapSpaceStatistics()) {

diff --git a/tools/snapshot/node_mksnapshot.cc b/tools/snapshot/node_mksnapshot.cc
@@ -7,7 +7,7 @@
 
 #include "libplatform/libplatform.h"
 #include "node_internals.h"
-#include "node_snapshotable.h"
+#include "node_snapshot_builder.h"
 #include "util-inl.h"
 #include "v8.h"