src: migrate from deprecated SnapshotCreator constructor

Previously we have been using the variant of SnapshotCreator that only passes the external references instead of v8::Isolate::CreateParams and it's about to be deprecated. Switch to using the new constructor that takes a fully CreateParams instead. This also makes sure that the snapshot building script is using the Node.js array buffer allocator instead of a separate default one that was previously used by the old constructor. The zero fill toggle in the Node.js array buffer allocator would still be ignored during snapshot building, however, until we fixes the array buffer allocator and let V8 own the toggle backing store instead, because otherwise the snapshot would contain the external toggle address and become unreproducible.
joyeecheung · Oct 9, 2024 · Apr 12, 2025 · Apr 14, 2025 · 7a4984f327cad2bb6da286f3d7d064d6465329c2
commit 7a4984f327cad2bb6da286f3d7d064d6465329c2
diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js
@@ -39,6 +39,13 @@ const {
   },
 } = internalBinding('util');
 
+const {
+  namespace: {
+    isBuildingSnapshot,
+  },
+  addAfterUserSerializeCallback,
+} = require('internal/v8/startup_snapshot');
+
 // Temporary buffers to convert numbers.
 const float32Array = new Float32Array(1);
 const uInt8Float32Array = new Uint8Array(float32Array.buffer);
@@ -1090,8 +1097,16 @@ function isMarkedAsUntransferable(obj) {
 // in C++.
 // |zeroFill| can be undefined when running inside an isolate where we
 // do not own the ArrayBuffer allocator.  Zero fill is always on in that case.
-let zeroFill = getZeroFillToggle();
+let zeroFill;
 function createUnsafeBuffer(size) {
+  if (!zeroFill) {
+    zeroFill = getZeroFillToggle();
+    if (isBuildingSnapshot()) {
+      addAfterUserSerializeCallback(() => {
+        zeroFill = undefined;
+      });
+    }
+  }
   zeroFill[0] = 0;
   try {
     return new FastBuffer(size);
@@ -1116,5 +1131,4 @@ module.exports = {
   createUnsafeBuffer,
   readUInt16BE,
   readUInt32BE,
-  reconnectZeroFillToggle,
 };
diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js
@@ -21,7 +21,6 @@ const {
   refreshOptions,
   getEmbedderOptions,
 } = require('internal/options');
-const { reconnectZeroFillToggle } = require('internal/buffer');
 const {
   exposeLazyInterfaces,
   defineReplaceableLazyAttribute,
@@ -91,7 +90,6 @@ function prepareExecution(options) {
   const { expandArgv1, initializeModules, isMainThread } = options;
 
   refreshRuntimeOptions();
-  reconnectZeroFillToggle();
 
   // Patch the process object and get the resolved main entry point.
   const mainEntry = patchProcessObject(expandArgv1);

diff --git a/src/api/embed_helpers.cc b/src/api/embed_helpers.cc
@@ -110,22 +110,36 @@ CommonEnvironmentSetup::CommonEnvironmentSetup(
   }
   loop->data = this;
 
+  impl_->allocator = ArrayBufferAllocator::Create();
+  const std::vector<intptr_t>& external_references =
+      SnapshotBuilder::CollectExternalReferences();
+  Isolate::CreateParams params;
+  params.array_buffer_allocator = impl_->allocator.get();
+  params.external_references = external_references.data();
   Isolate* isolate;
+
+  // Isolates created for snapshotting should be set up differently since
+  // it will be owned by the snapshot creator and needs to be cleaned up
+  // before serialization.
   if (flags & Flags::kIsForSnapshotting) {
-    const std::vector<intptr_t>& external_references =
-        SnapshotBuilder::CollectExternalReferences();
+    // The isolate must be registered before the SnapshotCreator initializes the
+    // isolate, so that the memory reducer can be initialized.
     isolate = impl_->isolate = Isolate::Allocate();
-    // Must be done before the SnapshotCreator creation so  that the
-    // memory reducer can be initialized.
     platform->RegisterIsolate(isolate, loop);
-    impl_->snapshot_creator.emplace(isolate, external_references.data());
+
+    impl_->snapshot_creator.emplace(isolate, params);
     isolate->SetCaptureStackTraceForUncaughtExceptions(
-        true, 10, v8::StackTrace::StackTraceOptions::kDetailed);
+        true,
+        static_cast<int>(
+            per_process::cli_options->per_isolate->stack_trace_limit),
+        v8::StackTrace::StackTraceOptions::kDetailed);
     SetIsolateMiscHandlers(isolate, {});
   } else {
-    impl_->allocator = ArrayBufferAllocator::Create();
     isolate = impl_->isolate =
-        NewIsolate(impl_->allocator, &impl_->loop, platform, snapshot_data);
+        NewIsolate(&params,
+                   &impl_->loop,
+                   platform,
+                   SnapshotData::FromEmbedderWrapper(snapshot_data));
   }
 
   {

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -1226,7 +1226,13 @@ void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
     // Create a dummy Uint32Array - the JS land can only toggle the C++ land
     // setting when the allocator uses our toggle. With this the toggle in JS
     // land results in no-ops.
+
+    ab = ArrayBuffer::New(env->isolate(), sizeof(uint32_t));
+  } else if (env->isolate_data()->is_building_snapshot()) {
     ab = ArrayBuffer::New(env->isolate(), sizeof(uint32_t));
+    // TODO(joyeecheung): save ab->GetBackingStore()->Data() in the Node.js
+    // array buffer allocator and include it into the C++ toggle while the
+    // Environment is still alive.
   } else {
     uint32_t* zero_fill_field = allocator->zero_fill_field();
     std::unique_ptr<BackingStore> backing =

diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc
@@ -863,9 +863,10 @@ const std::vector<intptr_t>& SnapshotBuilder::CollectExternalReferences() {
 
 void SnapshotBuilder::InitializeIsolateParams(const SnapshotData* data,
                                               Isolate::CreateParams* params) {
-  CHECK_NULL(params->external_references);
   CHECK_NULL(params->snapshot_blob);
-  params->external_references = CollectExternalReferences().data();
+  if (params->external_references == nullptr) {
+    params->external_references = CollectExternalReferences().data();
+  }
   params->snapshot_blob =
       const_cast<v8::StartupData*>(&(data->v8_snapshot_blob_data));
 }