Skip to content

Commit 97f59b9

Browse files
joyeecheungjoyeecheung
committed
buffer: move initialization of buffer prototype into node.js
Instead of exposing it in `lib/internal/buffer.js` after deleting it from the binding and then do the initialization in `lib/buffer.js`, which results in an implicit dependency on the order in which these modules are loaded. PR-URL: nodejs#25292 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent fa5af0d commit 97f59b9

5 files changed

Lines changed: 70 additions & 47 deletions

File tree

‎lib/buffer.js‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,22 @@ const {
3535
swap32: _swap32,
3636
swap64: _swap64,
3737
kMaxLength,
38-
kStringMaxLength
38+
kStringMaxLength,
39+
zeroFill: bindingZeroFill,
40+
41+
// Additional Buffer methods
42+
asciiSlice,
43+
base64Slice,
44+
latin1Slice,
45+
hexSlice,
46+
ucs2Slice,
47+
utf8Slice,
48+
asciiWrite,
49+
base64Write,
50+
latin1Write,
51+
hexWrite,
52+
ucs2Write,
53+
utf8Write
3954
} = internalBinding('buffer');
4055
const {
4156
getOwnNonIndexProperties,
@@ -74,10 +89,6 @@ const { validateString } = require('internal/validators');
7489

7590
const internalBuffer = require('internal/buffer');
7691

77-
const { setupBufferJS } = internalBuffer;
78-
79-
const bindingObj = {};
80-
8192
class FastBuffer extends Uint8Array {}
8293
FastBuffer.prototype.constructor = Buffer;
8394
internalBuffer.FastBuffer = FastBuffer;
@@ -88,6 +99,19 @@ for (const [name, method] of Object.entries(internalBuffer.readWrites)) {
8899
Buffer.prototype[name] = method;
89100
}
90101

102+
Buffer.prototype.asciiSlice = asciiSlice;
103+
Buffer.prototype.base64Slice = base64Slice;
104+
Buffer.prototype.latin1Slice = latin1Slice;
105+
Buffer.prototype.hexSlice = hexSlice;
106+
Buffer.prototype.ucs2Slice = ucs2Slice;
107+
Buffer.prototype.utf8Slice = utf8Slice;
108+
Buffer.prototype.asciiWrite = asciiWrite;
109+
Buffer.prototype.base64Write = base64Write;
110+
Buffer.prototype.latin1Write = latin1Write;
111+
Buffer.prototype.hexWrite = hexWrite;
112+
Buffer.prototype.ucs2Write = ucs2Write;
113+
Buffer.prototype.utf8Write = utf8Write;
114+
91115
const constants = Object.defineProperties({}, {
92116
MAX_LENGTH: {
93117
value: kMaxLength,
@@ -104,11 +128,11 @@ const constants = Object.defineProperties({}, {
104128
Buffer.poolSize = 8 * 1024;
105129
let poolSize, poolOffset, allocPool;
106130

107-
setupBufferJS(Buffer.prototype, bindingObj);
108-
131+
// A toggle used to access the zero fill setting of the array buffer allocator
132+
// in C++.
109133
// |zeroFill| can be undefined when running inside an isolate where we
110134
// do not own the ArrayBuffer allocator. Zero fill is always on in that case.
111-
const zeroFill = bindingObj.zeroFill || [0];
135+
const zeroFill = bindingZeroFill || [0];
112136

113137
function createUnsafeBuffer(size) {
114138
return new FastBuffer(createUnsafeArrayBuffer(size));

‎lib/internal/bootstrap/node.js‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -592,16 +592,21 @@ function setupGlobalVariables() {
592592
}
593593
});
594594

595-
// This, as side effect, removes `setupBufferJS` from the buffer binding,
596-
// and exposes it on `internal/buffer`.
597-
NativeModule.require('internal/buffer');
595+
const { Buffer } = NativeModule.require('buffer');
596+
const bufferBinding = internalBinding('buffer');
597+
598+
// Only after this point can C++ use Buffer::New()
599+
bufferBinding.setBufferPrototype(Buffer.prototype);
600+
delete bufferBinding.setBufferPrototype;
601+
delete bufferBinding.zeroFill;
598602

599603
Object.defineProperty(global, 'Buffer', {
600-
value: NativeModule.require('buffer').Buffer,
604+
value: Buffer,
601605
enumerable: false,
602606
writable: true,
603607
configurable: true
604608
});
609+
605610
process.domain = null;
606611
process._exiting = false;
607612
}

‎lib/internal/buffer.js‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,11 @@
11
'use strict';
22

3-
const binding = internalBinding('buffer');
43
const {
54
ERR_BUFFER_OUT_OF_BOUNDS,
65
ERR_INVALID_ARG_TYPE,
76
ERR_OUT_OF_RANGE
87
} = require('internal/errors').codes;
98
const { validateNumber } = require('internal/validators');
10-
const { setupBufferJS } = binding;
11-
12-
// Remove from the binding so that function is only available as exported here.
13-
// (That is, for internal use only.)
14-
delete binding.setupBufferJS;
159

1610
// Temporary buffers to convert numbers.
1711
const float32Array = new Float32Array(1);
@@ -779,7 +773,6 @@ function writeFloatBackwards(val, offset = 0) {
779773

780774
// FastBuffer wil be inserted here by lib/buffer.js
781775
module.exports = {
782-
setupBufferJS,
783776
// Container to export all read write functions.
784777
readWrites: {
785778
readUIntLE,

‎src/node_buffer.cc‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,38 +1057,12 @@ static void EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
10571057
}
10581058

10591059

1060-
// pass Buffer object to load prototype methods
1061-
void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
1060+
void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) {
10621061
Environment* env = Environment::GetCurrent(args);
10631062

10641063
CHECK(args[0]->IsObject());
10651064
Local<Object> proto = args[0].As<Object>();
10661065
env->set_buffer_prototype_object(proto);
1067-
1068-
env->SetMethodNoSideEffect(proto, "asciiSlice", StringSlice<ASCII>);
1069-
env->SetMethodNoSideEffect(proto, "base64Slice", StringSlice<BASE64>);
1070-
env->SetMethodNoSideEffect(proto, "latin1Slice", StringSlice<LATIN1>);
1071-
env->SetMethodNoSideEffect(proto, "hexSlice", StringSlice<HEX>);
1072-
env->SetMethodNoSideEffect(proto, "ucs2Slice", StringSlice<UCS2>);
1073-
env->SetMethodNoSideEffect(proto, "utf8Slice", StringSlice<UTF8>);
1074-
1075-
env->SetMethod(proto, "asciiWrite", StringWrite<ASCII>);
1076-
env->SetMethod(proto, "base64Write", StringWrite<BASE64>);
1077-
env->SetMethod(proto, "latin1Write", StringWrite<LATIN1>);
1078-
env->SetMethod(proto, "hexWrite", StringWrite<HEX>);
1079-
env->SetMethod(proto, "ucs2Write", StringWrite<UCS2>);
1080-
env->SetMethod(proto, "utf8Write", StringWrite<UTF8>);
1081-
1082-
if (auto zero_fill_field = env->isolate_data()->zero_fill_field()) {
1083-
CHECK(args[1]->IsObject());
1084-
auto binding_object = args[1].As<Object>();
1085-
auto array_buffer = ArrayBuffer::New(env->isolate(),
1086-
zero_fill_field,
1087-
sizeof(*zero_fill_field));
1088-
auto name = FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill");
1089-
auto value = Uint32Array::New(array_buffer, 0, 1);
1090-
CHECK(binding_object->Set(env->context(), name, value).FromJust());
1091-
}
10921066
}
10931067

10941068

@@ -1098,7 +1072,7 @@ void Initialize(Local<Object> target,
10981072
void* priv) {
10991073
Environment* env = Environment::GetCurrent(context);
11001074

1101-
env->SetMethod(target, "setupBufferJS", SetupBufferJS);
1075+
env->SetMethod(target, "setBufferPrototype", SetBufferPrototype);
11021076
env->SetMethodNoSideEffect(target, "createFromString", CreateFromString);
11031077

11041078
env->SetMethodNoSideEffect(target, "byteLengthUtf8", ByteLengthUtf8);
@@ -1123,6 +1097,32 @@ void Initialize(Local<Object> target,
11231097
target->Set(env->context(),
11241098
FIXED_ONE_BYTE_STRING(env->isolate(), "kStringMaxLength"),
11251099
Integer::New(env->isolate(), String::kMaxLength)).FromJust();
1100+
1101+
env->SetMethodNoSideEffect(target, "asciiSlice", StringSlice<ASCII>);
1102+
env->SetMethodNoSideEffect(target, "base64Slice", StringSlice<BASE64>);
1103+
env->SetMethodNoSideEffect(target, "latin1Slice", StringSlice<LATIN1>);
1104+
env->SetMethodNoSideEffect(target, "hexSlice", StringSlice<HEX>);
1105+
env->SetMethodNoSideEffect(target, "ucs2Slice", StringSlice<UCS2>);
1106+
env->SetMethodNoSideEffect(target, "utf8Slice", StringSlice<UTF8>);
1107+
1108+
env->SetMethod(target, "asciiWrite", StringWrite<ASCII>);
1109+
env->SetMethod(target, "base64Write", StringWrite<BASE64>);
1110+
env->SetMethod(target, "latin1Write", StringWrite<LATIN1>);
1111+
env->SetMethod(target, "hexWrite", StringWrite<HEX>);
1112+
env->SetMethod(target, "ucs2Write", StringWrite<UCS2>);
1113+
env->SetMethod(target, "utf8Write", StringWrite<UTF8>);
1114+
1115+
// It can be a nullptr when running inside an isolate where we
1116+
// do not own the ArrayBuffer allocator.
1117+
if (uint32_t* zero_fill_field = env->isolate_data()->zero_fill_field()) {
1118+
Local<ArrayBuffer> array_buffer = ArrayBuffer::New(
1119+
env->isolate(), zero_fill_field, sizeof(*zero_fill_field));
1120+
CHECK(target
1121+
->Set(env->context(),
1122+
FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill"),
1123+
Uint32Array::New(array_buffer, 0, 1))
1124+
.FromJust());
1125+
}
11261126
}
11271127

11281128
} // anonymous namespace

‎src/node_internals.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ v8::MaybeLocal<v8::Uint8Array> New(Environment* env,
246246
size_t byte_offset,
247247
size_t length) {
248248
v8::Local<v8::Uint8Array> ui = v8::Uint8Array::New(ab, byte_offset, length);
249+
CHECK(!env->buffer_prototype_object().IsEmpty());
249250
v8::Maybe<bool> mb =
250251
ui->SetPrototype(env->context(), env->buffer_prototype_object());
251252
if (mb.IsNothing())

0 commit comments

Comments
 (0)