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.

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

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

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 =

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