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: #25292 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
nodejs · Dec 31, 2018 · 042d20ab470dfe29b6f86b554ada41e5fc87700f · 042d20a
1 parent c718592
commit 042d20ab470dfe29b6f86b554ada41e5fc87700f
Unified Split

Showing with 69 additions and 47 deletions.

+32 −8 lib/buffer.js

+8 −4 lib/internal/bootstrap/node.js

+0 −7 lib/internal/buffer.js

+28 −28 src/node_buffer.cc

+1 −0 src/node_internals.h
diff --git a/lib/buffer.js b/lib/buffer.js
@@ -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,
@@ -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;
@@ -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,
@@ -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));

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -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;
 }

diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js
@@ -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);
@@ -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,

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -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());
-  
-  }
 }
@@ -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);
@@ -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

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -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())