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: https://github.com/nodejs/node/pull/25292
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

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,
@@ -74,10 +89,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;
@@ -88,6 +99,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,
@@ -104,11 +128,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));

lib/internal/bootstrap/node.js

@@ -592,16 +592,21 @@ function setupGlobalVariables() {
     }
   });
 
-  // This, as side effect, removes `setupBufferJS` from the buffer binding,
+  const { Buffer } = NativeModule.require('buffer');
-  // and exposes it on `internal/buffer`.
+  const bufferBinding = internalBinding('buffer');
-  NativeModule.require('internal/buffer');
+
+  // Only after this point can C++ use Buffer::New()
+  bufferBinding.setBufferPrototype(Buffer.prototype);
+  delete bufferBinding.setBufferPrototype;
+  delete bufferBinding.zeroFill;
 
   Object.defineProperty(global, 'Buffer', {
-    value: NativeModule.require('buffer').Buffer,
+    value: Buffer,
     enumerable: false,
     writable: true,
     configurable: true
   });
+
   process.domain = null;
   process._exiting = false;
 }

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,

src/node_buffer.cc

@@ -1057,38 +1057,12 @@ static void EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-// pass Buffer object to load prototype methods
+void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) {
-void SetupBufferJS(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());
-  }
 }
 
 
@@ -1098,7 +1072,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);
@@ -1123,6 +1097,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

src/node_internals.h

@@ -246,6 +246,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())