From dc29f45ad6dcca2ee3e70f438139ede15a3fbd4a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 7 May 2020 20:21:45 +0800 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/32984 Reviewed-By: Anna Henningsen Reviewed-By: Daniel Bevenius --- lib/buffer.js | 21 ++-------- lib/internal/bootstrap/pre_execution.js | 6 +++ lib/internal/buffer.js | 27 ++++++++++++- src/node_buffer.cc | 51 ++++++++++++++----------- 4 files changed, 64 insertions(+), 41 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 90ebe8866b582c..4119dd55eaacb7 100644 --- 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 index f1e2bcfb9da643..b384349666d919 100644 --- 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 index be509088ada1b6..aed6b24f292354 100644 --- 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 index 020b570cea3304..2028849775a98c 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -1117,6 +1117,34 @@ void SetBufferPrototype(const FunctionCallbackInfo& args) { env->set_buffer_prototype_object(proto); } +void GetZeroFillToggle(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator(); + Local 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 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 target, Local unused, @@ -1165,28 +1193,7 @@ void Initialize(Local target, env->SetMethod(target, "ucs2Write", StringWrite); env->SetMethod(target, "utf8Write", StringWrite); - // 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 backing = - ArrayBuffer::NewBackingStore(zero_fill_field, - sizeof(*zero_fill_field), - [](void*, size_t, void*){}, - nullptr); - Local 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