src: reset zero fill toggle at pre-execution

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 at pre-execution. PR-URL: #32984 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
nodejs · Jul 18, 2020 · c943cb4 · c943cb4
1 parent 0b8ae5f
commit c943cb4
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 41 deletions.
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -55,8 +55,7 @@ const {
   swap32: _swap32,
   swap64: _swap64,
   kMaxLength,
-  kStringMaxLength,
-  zeroFill: bindingZeroFill
+  kStringMaxLength
 } = internalBinding('buffer');
 const {
   getOwnNonIndexProperties,
@@ -102,7 +101,8 @@ const {
 const {
   FastBuffer,
   markAsUntransferable,
-  addBufferPrototypeMethods
+  addBufferPrototypeMethods,
+  createUnsafeBuffer
 } = require('internal/buffer');
 
 const TypedArrayPrototype = ObjectGetPrototypeOf(Uint8ArrayPrototype);
@@ -132,25 +132,10 @@ const constants = ObjectDefineProperties({}, {
 Buffer.poolSize = 8 * 1024;
 let poolSize, poolOffset, allocPool;
 
-// A toggle used to access the zero fill setting of the array buffer allocator
-// 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.
-const zeroFill = bindingZeroFill || [0];
-
 const encodingsMap = ObjectCreate(null);
 for (let i = 0; i < encodings.length; ++i)
   encodingsMap[encodings[i]] = i;
 
-function createUnsafeBuffer(size) {
-  zeroFill[0] = 0;
-  try {
-    return new FastBuffer(size);
-  } finally {
-    zeroFill[0] = 1;
-  }
-}
-
 function createPool() {
   poolSize = Buffer.poolSize;
   allocPool = createUnsafeBuffer(poolSize).buffer;

diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -10,11 +10,17 @@ const {
   getOptionValue,
   shouldNotRegisterESMLoader
 } = require('internal/options');
+const { reconnectZeroFillToggle } = require('internal/buffer');
+
 const { Buffer } = require('buffer');
 const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes;
 const assert = require('internal/assert');
 
 function prepareMainThreadExecution(expandArgv1 = false) {
+  // TODO(joyeecheung): this is also necessary for workers when they deserialize
+  // this toggle from the snapshot.
+  reconnectZeroFillToggle();
+
   // Patch the process object with legacy properties and normalizations
   patchProcessObject(expandArgv1);
   setupTraceCategoryState();

diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js
@@ -25,7 +25,8 @@ const {
   latin1Write,
   hexWrite,
   ucs2Write,
-  utf8Write
+  utf8Write,
+  getZeroFillToggle
 } = internalBinding('buffer');
 const {
   untransferable_object_private_symbol,
@@ -1019,8 +1020,32 @@ function markAsUntransferable(obj) {
   setHiddenValue(obj, untransferable_object_private_symbol, true);
 }
 
+// A toggle used to access the zero fill setting of the array buffer allocator
+// 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();
+function createUnsafeBuffer(size) {
+  zeroFill[0] = 0;
+  try {
+    return new FastBuffer(size);
+  } finally {
+    zeroFill[0] = 1;
+  }
+}
+
+// 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,
   markAsUntransferable,
+  createUnsafeBuffer,
+  reconnectZeroFillToggle
 };
diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -1117,6 +1117,34 @@ void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) {
   env->set_buffer_prototype_object(proto);
 }
 
+void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator();
+  Local<ArrayBuffer> ab;
+  // It can be a nullptr when running inside an isolate where we
+  // do not own the ArrayBuffer allocator.
+  if (allocator == nullptr) {
+    // 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 {
+    uint32_t* zero_fill_field = allocator->zero_fill_field();
+    std::unique_ptr<BackingStore> backing =
+        ArrayBuffer::NewBackingStore(zero_fill_field,
+                                     sizeof(*zero_fill_field),
+                                     [](void*, size_t, void*) {},
+                                     nullptr);
+    ab = ArrayBuffer::New(env->isolate(), std::move(backing));
+  }
+
+  ab->SetPrivate(
+      env->context(),
+      env->untransferable_object_private_symbol(),
+      True(env->isolate())).Check();
+
+  args.GetReturnValue().Set(Uint32Array::New(ab, 0, 1));
+}
 
 void Initialize(Local<Object> target,
                 Local<Value> unused,
@@ -1165,28 +1193,7 @@ void Initialize(Local<Object> target,
   env->SetMethod(target, "ucs2Write", StringWrite<UCS2>);
   env->SetMethod(target, "utf8Write", StringWrite<UTF8>);
 
-  // It can be a nullptr when running inside an isolate where we
-  // do not own the ArrayBuffer allocator.
-  if (NodeArrayBufferAllocator* allocator =
-          env->isolate_data()->node_allocator()) {
-    uint32_t* zero_fill_field = allocator->zero_fill_field();
-    std::unique_ptr<BackingStore> backing =
-      ArrayBuffer::NewBackingStore(zero_fill_field,
-                                   sizeof(*zero_fill_field),
-                                   [](void*, size_t, void*){},
-                                   nullptr);
-    Local<ArrayBuffer> array_buffer =
-        ArrayBuffer::New(env->isolate(), std::move(backing));
-    array_buffer->SetPrivate(
-        env->context(),
-        env->untransferable_object_private_symbol(),
-        True(env->isolate())).Check();
-    CHECK(target
-              ->Set(env->context(),
-                    FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill"),
-                    Uint32Array::New(array_buffer, 0, 1))
-              .FromJust());
-  }
+  env->SetMethod(target, "getZeroFillToggle", GetZeroFillToggle);
 }
 
 }  // anonymous namespace