Skip to content

Commit

Permalink
buffer: move initialization of buffer prototype into node.js
Browse files Browse the repository at this point in the history
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: #25292
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
joyeecheung authored and BridgeAR committed Jan 17, 2019
1 parent c718592 commit 042d20a
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 47 deletions.
40 changes: 32 additions & 8 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,22 @@ const {
swap32: _swap32,
swap64: _swap64,
kMaxLength,
kStringMaxLength
kStringMaxLength,
zeroFill: bindingZeroFill,

// Additional Buffer methods
asciiSlice,
base64Slice,
latin1Slice,
hexSlice,
ucs2Slice,
utf8Slice,
asciiWrite,
base64Write,
latin1Write,
hexWrite,
ucs2Write,
utf8Write
} = internalBinding('buffer');
const {
getOwnNonIndexProperties,
Expand Down Expand Up @@ -75,10 +90,6 @@ const { validateString } = require('internal/validators');

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

const { setupBufferJS } = internalBuffer;

const bindingObj = {};

class FastBuffer extends Uint8Array {}
FastBuffer.prototype.constructor = Buffer;
internalBuffer.FastBuffer = FastBuffer;
Expand All @@ -89,6 +100,19 @@ for (const [name, method] of Object.entries(internalBuffer.readWrites)) {
Buffer.prototype[name] = method;
}

Buffer.prototype.asciiSlice = asciiSlice;
Buffer.prototype.base64Slice = base64Slice;
Buffer.prototype.latin1Slice = latin1Slice;
Buffer.prototype.hexSlice = hexSlice;
Buffer.prototype.ucs2Slice = ucs2Slice;
Buffer.prototype.utf8Slice = utf8Slice;
Buffer.prototype.asciiWrite = asciiWrite;
Buffer.prototype.base64Write = base64Write;
Buffer.prototype.latin1Write = latin1Write;
Buffer.prototype.hexWrite = hexWrite;
Buffer.prototype.ucs2Write = ucs2Write;
Buffer.prototype.utf8Write = utf8Write;

const constants = Object.defineProperties({}, {
MAX_LENGTH: {
value: kMaxLength,
Expand All @@ -105,11 +129,11 @@ const constants = Object.defineProperties({}, {
Buffer.poolSize = 8 * 1024;
let poolSize, poolOffset, allocPool;

setupBufferJS(Buffer.prototype, bindingObj);

// 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 = bindingObj.zeroFill || [0];
const zeroFill = bindingZeroFill || [0];

function createUnsafeBuffer(size) {
return new FastBuffer(createUnsafeArrayBuffer(size));
Expand Down
12 changes: 8 additions & 4 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -653,11 +653,15 @@ function setupGlobalVariables() {
}
});

// This, as side effect, removes `setupBufferJS` from the buffer binding,
// and exposes it on `internal/buffer`.
NativeModule.require('internal/buffer');
const { Buffer } = NativeModule.require('buffer');
const bufferBinding = internalBinding('buffer');

global.Buffer = NativeModule.require('buffer').Buffer;
// Only after this point can C++ use Buffer::New()
bufferBinding.setBufferPrototype(Buffer.prototype);
delete bufferBinding.setBufferPrototype;
delete bufferBinding.zeroFill;

global.Buffer = Buffer;
process.domain = null;
process._exiting = false;
}
Expand Down
7 changes: 0 additions & 7 deletions lib/internal/buffer.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
'use strict';

const binding = internalBinding('buffer');
const {
ERR_BUFFER_OUT_OF_BOUNDS,
ERR_INVALID_ARG_TYPE,
ERR_OUT_OF_RANGE
} = require('internal/errors').codes;
const { validateNumber } = require('internal/validators');
const { setupBufferJS } = binding;

// Remove from the binding so that function is only available as exported here.
// (That is, for internal use only.)
delete binding.setupBufferJS;

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

// FastBuffer wil be inserted here by lib/buffer.js
module.exports = {
setupBufferJS,
// Container to export all read write functions.
readWrites: {
readUIntLE,
Expand Down
56 changes: 28 additions & 28 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1055,38 +1055,12 @@ static void EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
}


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

CHECK(args[0]->IsObject());
Local<Object> proto = args[0].As<Object>();
env->set_buffer_prototype_object(proto);

env->SetMethodNoSideEffect(proto, "asciiSlice", StringSlice<ASCII>);
env->SetMethodNoSideEffect(proto, "base64Slice", StringSlice<BASE64>);
env->SetMethodNoSideEffect(proto, "latin1Slice", StringSlice<LATIN1>);
env->SetMethodNoSideEffect(proto, "hexSlice", StringSlice<HEX>);
env->SetMethodNoSideEffect(proto, "ucs2Slice", StringSlice<UCS2>);
env->SetMethodNoSideEffect(proto, "utf8Slice", StringSlice<UTF8>);

env->SetMethod(proto, "asciiWrite", StringWrite<ASCII>);
env->SetMethod(proto, "base64Write", StringWrite<BASE64>);
env->SetMethod(proto, "latin1Write", StringWrite<LATIN1>);
env->SetMethod(proto, "hexWrite", StringWrite<HEX>);
env->SetMethod(proto, "ucs2Write", StringWrite<UCS2>);
env->SetMethod(proto, "utf8Write", StringWrite<UTF8>);

if (auto zero_fill_field = env->isolate_data()->zero_fill_field()) {
CHECK(args[1]->IsObject());
auto binding_object = args[1].As<Object>();
auto array_buffer = ArrayBuffer::New(env->isolate(),
zero_fill_field,
sizeof(*zero_fill_field));
auto name = FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill");
auto value = Uint32Array::New(array_buffer, 0, 1);
CHECK(binding_object->Set(env->context(), name, value).FromJust());
}
}


Expand All @@ -1096,7 +1070,7 @@ void Initialize(Local<Object> target,
void* priv) {
Environment* env = Environment::GetCurrent(context);

env->SetMethod(target, "setupBufferJS", SetupBufferJS);
env->SetMethod(target, "setBufferPrototype", SetBufferPrototype);
env->SetMethodNoSideEffect(target, "createFromString", CreateFromString);

env->SetMethodNoSideEffect(target, "byteLengthUtf8", ByteLengthUtf8);
Expand All @@ -1121,6 +1095,32 @@ void Initialize(Local<Object> target,
target->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "kStringMaxLength"),
Integer::New(env->isolate(), String::kMaxLength)).FromJust();

env->SetMethodNoSideEffect(target, "asciiSlice", StringSlice<ASCII>);
env->SetMethodNoSideEffect(target, "base64Slice", StringSlice<BASE64>);
env->SetMethodNoSideEffect(target, "latin1Slice", StringSlice<LATIN1>);
env->SetMethodNoSideEffect(target, "hexSlice", StringSlice<HEX>);
env->SetMethodNoSideEffect(target, "ucs2Slice", StringSlice<UCS2>);
env->SetMethodNoSideEffect(target, "utf8Slice", StringSlice<UTF8>);

env->SetMethod(target, "asciiWrite", StringWrite<ASCII>);
env->SetMethod(target, "base64Write", StringWrite<BASE64>);
env->SetMethod(target, "latin1Write", StringWrite<LATIN1>);
env->SetMethod(target, "hexWrite", StringWrite<HEX>);
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 (uint32_t* zero_fill_field = env->isolate_data()->zero_fill_field()) {
Local<ArrayBuffer> array_buffer = ArrayBuffer::New(
env->isolate(), zero_fill_field, sizeof(*zero_fill_field));
CHECK(target
->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "zeroFill"),
Uint32Array::New(array_buffer, 0, 1))
.FromJust());
}
}

} // anonymous namespace
Expand Down
1 change: 1 addition & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ v8::MaybeLocal<v8::Uint8Array> New(Environment* env,
size_t byte_offset,
size_t length) {
v8::Local<v8::Uint8Array> ui = v8::Uint8Array::New(ab, byte_offset, length);
CHECK(!env->buffer_prototype_object().IsEmpty());
v8::Maybe<bool> mb =
ui->SetPrototype(env->context(), env->buffer_prototype_object());
if (mb.IsNothing())
Expand Down

0 comments on commit 042d20a

Please sign in to comment.