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.

PR-URL: #55337
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Vladimir Morozov <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: joyeecheung

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,18 @@ 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()) {
+      // Reset the toggle so that after serialization, we'll re-create a real
+      // toggle connected to the C++ one via getZeroFillToggle().
+      addAfterUserSerializeCallback(() => {
+        zeroFill = undefined;
+      });
+    }
+  }
   zeroFill[0] = 0;
   try {
     return new FastBuffer(size);
@@ -1100,14 +1117,6 @@ function createUnsafeBuffer(size) {
   }
 }
 
-// The connection between the JS land zero fill toggle and the
-// C++ one in the NodeArrayBufferAllocator gets lost if the toggle
-// is deserialized from the snapshot, because V8 owns the underlying
-// memory of this toggle. This resets the connection.
-function reconnectZeroFillToggle() {
-  zeroFill = getZeroFillToggle();
-}
-
 module.exports = {
   FastBuffer,
   addBufferPrototypeMethods,
@@ -1116,5 +1125,4 @@ module.exports = {
   createUnsafeBuffer,
   readUInt16BE,
   readUInt32BE,
-  reconnectZeroFillToggle,
 };

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);

src/api/embed_helpers.cc

@@ -110,22 +110,40 @@ 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();
+  params.external_references = external_references.data();
+  params.cpp_heap =
+      v8::CppHeap::Create(platform, v8::CppHeapCreateParams{{}}).release();
+
   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));
   }
 
   {
@@ -220,10 +238,10 @@ CommonEnvironmentSetup::~CommonEnvironmentSetup() {
       *static_cast<bool*>(data) = true;
     }, &platform_finished);
     impl_->platform->UnregisterIsolate(isolate);
-    if (impl_->snapshot_creator.has_value())
+    if (impl_->snapshot_creator.has_value()) {
       impl_->snapshot_creator.reset();
-    else
-      isolate->Dispose();
+    }
+    isolate->Dispose();
 
     // Wait until the platform has cleaned up all relevant resources.
     while (!platform_finished)

src/node_buffer.cc

@@ -1222,12 +1222,20 @@ void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
   Local<ArrayBuffer> ab;
   // It can be a nullptr when running inside an isolate where we
   // do not own the ArrayBuffer allocator.
-  if (allocator == nullptr) {
+  if (allocator == nullptr || env->isolate_data()->is_building_snapshot()) {
     // 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.
+    // When building a snapshot, just use a dummy toggle as well to avoid
+    // introducing the dynamic external reference. We'll re-initialize the
+    // toggle with a real one connected to the C++ allocator after snapshot
+    // deserialization.
+
     ab = ArrayBuffer::New(env->isolate(), sizeof(uint32_t));
   } else {
+    // 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.
     uint32_t* zero_fill_field = allocator->zero_fill_field();
     std::unique_ptr<BackingStore> backing =
         ArrayBuffer::NewBackingStore(zero_fill_field,

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));
 }