Skip to content

Commit c943cb4

Browse files
committed
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 <[email protected]> Reviewed-By: Daniel Bevenius <[email protected]>
1 parent 0b8ae5f commit c943cb4

File tree

4 files changed

+64
-41
lines changed

4 files changed

+64
-41
lines changed

Diff for: lib/buffer.js

+3-18
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ const {
5555
swap32: _swap32,
5656
swap64: _swap64,
5757
kMaxLength,
58-
kStringMaxLength,
59-
zeroFill: bindingZeroFill
58+
kStringMaxLength
6059
} = internalBinding('buffer');
6160
const {
6261
getOwnNonIndexProperties,
@@ -102,7 +101,8 @@ const {
102101
const {
103102
FastBuffer,
104103
markAsUntransferable,
105-
addBufferPrototypeMethods
104+
addBufferPrototypeMethods,
105+
createUnsafeBuffer
106106
} = require('internal/buffer');
107107

108108
const TypedArrayPrototype = ObjectGetPrototypeOf(Uint8ArrayPrototype);
@@ -132,25 +132,10 @@ const constants = ObjectDefineProperties({}, {
132132
Buffer.poolSize = 8 * 1024;
133133
let poolSize, poolOffset, allocPool;
134134

135-
// A toggle used to access the zero fill setting of the array buffer allocator
136-
// in C++.
137-
// |zeroFill| can be undefined when running inside an isolate where we
138-
// do not own the ArrayBuffer allocator. Zero fill is always on in that case.
139-
const zeroFill = bindingZeroFill || [0];
140-
141135
const encodingsMap = ObjectCreate(null);
142136
for (let i = 0; i < encodings.length; ++i)
143137
encodingsMap[encodings[i]] = i;
144138

145-
function createUnsafeBuffer(size) {
146-
zeroFill[0] = 0;
147-
try {
148-
return new FastBuffer(size);
149-
} finally {
150-
zeroFill[0] = 1;
151-
}
152-
}
153-
154139
function createPool() {
155140
poolSize = Buffer.poolSize;
156141
allocPool = createUnsafeBuffer(poolSize).buffer;

Diff for: lib/internal/bootstrap/pre_execution.js

+6
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,17 @@ const {
1010
getOptionValue,
1111
shouldNotRegisterESMLoader
1212
} = require('internal/options');
13+
const { reconnectZeroFillToggle } = require('internal/buffer');
14+
1315
const { Buffer } = require('buffer');
1416
const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes;
1517
const assert = require('internal/assert');
1618

1719
function prepareMainThreadExecution(expandArgv1 = false) {
20+
// TODO(joyeecheung): this is also necessary for workers when they deserialize
21+
// this toggle from the snapshot.
22+
reconnectZeroFillToggle();
23+
1824
// Patch the process object with legacy properties and normalizations
1925
patchProcessObject(expandArgv1);
2026
setupTraceCategoryState();

Diff for: lib/internal/buffer.js

+26-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ const {
2525
latin1Write,
2626
hexWrite,
2727
ucs2Write,
28-
utf8Write
28+
utf8Write,
29+
getZeroFillToggle
2930
} = internalBinding('buffer');
3031
const {
3132
untransferable_object_private_symbol,
@@ -1019,8 +1020,32 @@ function markAsUntransferable(obj) {
10191020
setHiddenValue(obj, untransferable_object_private_symbol, true);
10201021
}
10211022

1023+
// A toggle used to access the zero fill setting of the array buffer allocator
1024+
// in C++.
1025+
// |zeroFill| can be undefined when running inside an isolate where we
1026+
// do not own the ArrayBuffer allocator. Zero fill is always on in that case.
1027+
let zeroFill = getZeroFillToggle();
1028+
function createUnsafeBuffer(size) {
1029+
zeroFill[0] = 0;
1030+
try {
1031+
return new FastBuffer(size);
1032+
} finally {
1033+
zeroFill[0] = 1;
1034+
}
1035+
}
1036+
1037+
// The connection between the JS land zero fill toggle and the
1038+
// C++ one in the NodeArrayBufferAllocator gets lost if the toggle
1039+
// is deserialized from the snapshot, because V8 owns the underlying
1040+
// memory of this toggle. This resets the connection.
1041+
function reconnectZeroFillToggle() {
1042+
zeroFill = getZeroFillToggle();
1043+
}
1044+
10221045
module.exports = {
10231046
FastBuffer,
10241047
addBufferPrototypeMethods,
10251048
markAsUntransferable,
1049+
createUnsafeBuffer,
1050+
reconnectZeroFillToggle
10261051
};

Diff for: src/node_buffer.cc

+29-22
Original file line numberDiff line numberDiff line change
@@ -1117,6 +1117,34 @@ void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) {
11171117
env->set_buffer_prototype_object(proto);
11181118
}
11191119

1120+
void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
1121+
Environment* env = Environment::GetCurrent(args);
1122+
NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator();
1123+
Local<ArrayBuffer> ab;
1124+
// It can be a nullptr when running inside an isolate where we
1125+
// do not own the ArrayBuffer allocator.
1126+
if (allocator == nullptr) {
1127+
// Create a dummy Uint32Array - the JS land can only toggle the C++ land
1128+
// setting when the allocator uses our toggle. With this the toggle in JS
1129+
// land results in no-ops.
1130+
ab = ArrayBuffer::New(env->isolate(), sizeof(uint32_t));
1131+
} else {
1132+
uint32_t* zero_fill_field = allocator->zero_fill_field();
1133+
std::unique_ptr<BackingStore> backing =
1134+
ArrayBuffer::NewBackingStore(zero_fill_field,
1135+
sizeof(*zero_fill_field),
1136+
[](void*, size_t, void*) {},
1137+
nullptr);
1138+
ab = ArrayBuffer::New(env->isolate(), std::move(backing));
1139+
}
1140+
1141+
ab->SetPrivate(
1142+
env->context(),
1143+
env->untransferable_object_private_symbol(),
1144+
True(env->isolate())).Check();
1145+
1146+
args.GetReturnValue().Set(Uint32Array::New(ab, 0, 1));
1147+
}
11201148

11211149
void Initialize(Local<Object> target,
11221150
Local<Value> unused,
@@ -1165,28 +1193,7 @@ void Initialize(Local<Object> target,
11651193
env->SetMethod(target, "ucs2Write", StringWrite<UCS2>);
11661194
env->SetMethod(target, "utf8Write", StringWrite<UTF8>);
11671195

1168-
// It can be a nullptr when running inside an isolate where we
1169-
// do not own the ArrayBuffer allocator.
1170-
if (NodeArrayBufferAllocator* allocator =
1171-
env->isolate_data()->node_allocator()) {
1172-
uint32_t* zero_fill_field = allocator->zero_fill_field();
1173-
std::unique_ptr<BackingStore> backing =
1174-
ArrayBuffer::NewBackingStore(zero_fill_field,
1175-
sizeof(*zero_fill_field),
1176-
[](void*, size_t, void*){},
1177-
nullptr);
1178-
Local<ArrayBuffer> array_buffer =
1179-
ArrayBuffer::New(env->isolate(), std::move(backing));
1180-
array_buffer->SetPrivate(
1181-
env->context(),
1182-
env->untransferable_object_private_symbol(),
1183-
True(env->isolate())).Check();
1184-
CHECK(target
1185-
->Set(env->context(),
1186-
FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill"),
1187-
Uint32Array::New(array_buffer, 0, 1))
1188-
.FromJust());
1189-
}
1196+
env->SetMethod(target, "getZeroFillToggle", GetZeroFillToggle);
11901197
}
11911198

11921199
} // anonymous namespace

0 commit comments

Comments
 (0)