Skip to content

Commit

Permalink
src: reset zero fill toggle at pre-execution
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
joyeecheung authored and cjihrig committed Jul 22, 2020
1 parent 062f3c7 commit dc29f45
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 41 deletions.
21 changes: 3 additions & 18 deletions lib/buffer.js
Expand Up @@ -55,8 +55,7 @@ const {
swap32: _swap32,
swap64: _swap64,
kMaxLength,
kStringMaxLength,
zeroFill: bindingZeroFill
kStringMaxLength
} = internalBinding('buffer');
const {
getOwnNonIndexProperties,
Expand Down Expand Up @@ -102,7 +101,8 @@ const {
const {
FastBuffer,
markAsUntransferable,
addBufferPrototypeMethods
addBufferPrototypeMethods,
createUnsafeBuffer
} = require('internal/buffer');

const TypedArrayPrototype = ObjectGetPrototypeOf(Uint8ArrayPrototype);
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions lib/internal/bootstrap/pre_execution.js
Expand Up @@ -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();
Expand Down
27 changes: 26 additions & 1 deletion lib/internal/buffer.js
Expand Up @@ -25,7 +25,8 @@ const {
latin1Write,
hexWrite,
ucs2Write,
utf8Write
utf8Write,
getZeroFillToggle
} = internalBinding('buffer');
const {
untransferable_object_private_symbol,
Expand Down Expand Up @@ -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
};
51 changes: 29 additions & 22 deletions src/node_buffer.cc
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit dc29f45

Please sign in to comment.