buffer: add Buffer.allocUnsafeSlow(size)

Aligns the functionality of SlowBuffer with the new Buffer
constructor API. Next step is to docs-only deprecate
SlowBuffer.

Replace the internal uses of SlowBuffer with
`Buffer.allocUnsafeSlow(size)`

PR-URL: #5833
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>

Author: jasnell

benchmark/buffers/buffer-creation.js

@@ -8,6 +8,7 @@ const bench = common.createBenchmark(main, {
     'fast-alloc',
     'fast-alloc-fill',
     'fast-allocUnsafe',
+    'slow-allocUnsafe',
     'slow',
     'buffer()'],
   len: [10, 1024, 2048, 4096, 8192],
@@ -39,6 +40,13 @@ function main(conf) {
       }
       bench.end(n);
       break;
+    case 'slow-allocUnsafe':
+      bench.start();
+      for (let i = 0; i < n * 1024; i++) {
+        Buffer.allocUnsafeSlow(len);
+      }
+      bench.end(n);
+      break;
     case 'slow':
       bench.start();
       for (let i = 0; i < n * 1024; i++) {

doc/api/buffer.markdown

@@ -87,42 +87,45 @@ to one of these new APIs.*
   containing a *copy* of the provided string.
 * [`Buffer.alloc(size[, fill[, encoding]])`][buffer_alloc] returns a "filled"
   `Buffer` instance of the specified size. This method can be significantly
-  slower than [`Buffer.allocUnsafe(size)`][buffer_allocunsafe] but ensures that
-  newly created `Buffer` instances never contain old and potentially sensitive
-  data.
-* [`Buffer.allocUnsafe(size)`][buffer_allocunsafe] returns a new `Buffer` of
-  the specified `size` whose content *must* be initialized using either
-  [`buf.fill(0)`][] or written to completely.
+  slower than [`Buffer.allocUnsafe(size)`][buffer_allocunsafe] but ensures
+  that newly created `Buffer` instances never contain old and potentially
+  sensitive data.
+* [`Buffer.allocUnsafe(size)`][buffer_allocunsafe] and
+  [`Buffer.allocUnsafeSlow(size)`][buffer_allocunsafeslow] each return a
+  new `Buffer` of the specified `size` whose content *must* be initialized
+  using either [`buf.fill(0)`][] or written to completely.
 
 `Buffer` instances returned by `Buffer.allocUnsafe(size)` *may* be allocated
-off a shared internal memory pool if the `size` is less than or equal to half
-`Buffer.poolSize`.
+off a shared internal memory pool if `size` is less than or equal to half
+`Buffer.poolSize`. Instances returned by `Buffer.allocUnsafeSlow(size)` *never*
+use the shared internal memory pool.
 
 ### The `--zero-fill-buffers` command line option
 
 Node.js can be started using the `--zero-fill-buffers` command line option to
 force all newly allocated `Buffer` and `SlowBuffer` instances created using
-either `new Buffer(size)`, `Buffer.allocUnsafe(size)`, or
-`new SlowBuffer(size)` to be *automatically zero-filled* upon creation. Use of
-this flag *changes the default behavior* of these methods and *can have a
-significant impact* on performance. Use of the `--zero-fill-buffers` option is
-recommended only when absolutely necessary to enforce that newly allocated
-`Buffer` instances cannot contain potentially sensitive data.
+either `new Buffer(size)`, `Buffer.allocUnsafe(size)`,
+`Buffer.allocUnsafeSlow(size)` or `new SlowBuffer(size)` to be *automatically
+zero-filled* upon creation. Use of this flag *changes the default behavior* of
+these methods and *can have a significant impact* on performance. Use of the
+`--zero-fill-buffers` option is recommended only when absolutely necessary to
+enforce that newly allocated `Buffer` instances cannot contain potentially
+sensitive data.
 
 ```
 $ node --zero-fill-buffers
 > Buffer.allocUnsafe(5);
 <Buffer 00 00 00 00 00>
 ```
 
-### What makes `Buffer.allocUnsafe(size)` "unsafe"?
+### What makes `Buffer.allocUnsafe(size)` and `Buffer.allocUnsafeSlow(size)` "unsafe"?
 
-When calling `Buffer.allocUnsafe()`, the segment of allocated memory is
-*uninitialized* (it is not zeroed-out). While this design makes the allocation
-of memory quite fast, the allocated segment of memory might contain old data
-that is potentially sensitive. Using a `Buffer` created by
-`Buffer.allocUnsafe(size)` without *completely* overwriting the memory can
-allow this old data to be leaked when the `Buffer` memory is read.
+When calling `Buffer.allocUnsafe()` (and `Buffer.allocUnsafeSlow()`), the
+segment of allocated memory is *uninitialized* (it is not zeroed-out). While
+this design makes the allocation of memory quite fast, the allocated segment of
+memory might contain old data that is potentially sensitive. Using a `Buffer`
+created by `Buffer.allocUnsafe()` without *completely* overwriting the memory
+can allow this old data to be leaked when the `Buffer` memory is read.
 
 While there are clear performance advantages to using `Buffer.allocUnsafe()`,
 extra care *must* be taken in order to avoid introducing security
@@ -466,6 +469,52 @@ Buffer pool if `size` is less than or equal to half `Buffer.poolSize`. The
 difference is subtle but can be important when an application requires the
 additional performance that `Buffer.allocUnsafe(size)` provides.
 
+### Class Method: Buffer.allocUnsafeSlow(size)
+
+* `size` {Number}
+
+Allocates a new *non-zero-filled* and non-pooled `Buffer` of `size` bytes.  The
+`size` must be less than or equal to the value of
+`require('buffer').kMaxLength` (on 64-bit architectures, `kMaxLength` is
+`(2^31)-1`). Otherwise, a [`RangeError`][] is thrown. If a `size` less than 0
+is specified, a zero-length `Buffer` will be created.
+
+The underlying memory for `Buffer` instances created in this way is *not
+initialized*. The contents of the newly created `Buffer` are unknown and
+*may contain sensitive data*. Use [`buf.fill(0)`][] to initialize such
+`Buffer` instances to zeroes.
+
+When using `Buffer.allocUnsafe()` to allocate new `Buffer` instances,
+allocations under 4KB are, by default, sliced from a single pre-allocated
+`Buffer`. This allows applications to avoid the garbage collection overhead of
+creating many individually allocated Buffers. This approach improves both
+performance and memory usage by eliminating the need to track and cleanup as
+many `Persistent` objects.
+
+However, in the case where a developer may need to retain a small chunk of
+memory from a pool for an indeterminate amount of time, it may be appropriate
+to create an un-pooled Buffer instance using `Buffer.allocUnsafeSlow()` then
+copy out the relevant bits.
+
+```js
+// need to keep around a few small chunks of memory
+const store = [];
+
+socket.on('readable', () => {
+  const data = socket.read();
+  // allocate for retained data
+  const sb = Buffer.allocUnsafeSlow(10);
+  // copy the data into the new allocation
+  data.copy(sb, 0, 0, 10);
+  store.push(sb);
+});
+```
+
+Use of `Buffer.allocUnsafeSlow()` should be used only as a last resort *after*
+a developer has observed undue memory retention in their applications.
+
+A `TypeError` will be thrown if `size` is not a number.
+
 ### Class Method: Buffer.byteLength(string[, encoding])
 
 * `string` {String | Buffer | TypedArray | DataView | ArrayBuffer}
@@ -1805,7 +1854,8 @@ console.log(buf);
 [buffer_from_buffer]: #buffer_class_method_buffer_from_buffer
 [buffer_from_arraybuf]: #buffer_class_method_buffer_from_arraybuffer
 [buffer_from_string]: #buffer_class_method_buffer_from_str_encoding
-[buffer_allocunsafe]: #buffer_class_method_buffer_allocraw_size
+[buffer_allocunsafe]: #buffer_class_method_buffer_allocunsafe_size
+[buffer_allocunsafeslow]: #buffer_class_method_buffer_allocunsafeslow_size
 [buffer_alloc]: #buffer_class_method_buffer_alloc_size_fill_encoding
 [`TypedArray.from()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/from
 [`DataView`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DataView

lib/buffer.js

@@ -133,13 +133,23 @@ Buffer.from = function(value, encodingOrOffset, length) {
 Object.setPrototypeOf(Buffer.prototype, Uint8Array.prototype);
 Object.setPrototypeOf(Buffer, Uint8Array);
 
+function assertSize(size) {
+  if (typeof size !== 'number') {
+    const err = new TypeError('"size" argument must be a number');
+    // The following hides the 'assertSize' method from the
+    // callstack. This is done simply to hide the internal
+    // details of the implementation from bleeding out to users.
+    Error.captureStackTrace(err, assertSize);
+    throw err;
+  }
+}
+
 /**
  * Creates a new filled Buffer instance.
  * alloc(size[, fill[, encoding]])
  **/
 Buffer.alloc = function(size, fill, encoding) {
-  if (typeof size !== 'number')
-    throw new TypeError('"size" argument must be a number');
+  assertSize(size);
   if (size <= 0)
     return createBuffer(size);
   if (fill !== undefined) {
@@ -161,11 +171,22 @@ Buffer.alloc = function(size, fill, encoding) {
  * instance. If `--zero-fill-buffers` is set, will zero-fill the buffer.
  **/
 Buffer.allocUnsafe = function(size) {
-  if (typeof size !== 'number')
-    throw new TypeError('"size" argument must be a number');
+  assertSize(size);
   return allocate(size);
 };
 
+/**
+ * Equivalent to SlowBuffer(num), by default creates a non-zero-filled
+ * Buffer instance that is not allocated off the pre-initialized pool.
+ * If `--zero-fill-buffers` is set, will zero-fill the buffer.
+ **/
+Buffer.allocUnsafeSlow = function(size) {
+  assertSize(size);
+  if (size > 0)
+    flags[kNoZeroFill] = 1;
+  return createBuffer(size);
+};
+
 // If --zero-fill-buffers command line argument is set, a zero-filled
 // buffer is returned.
 function SlowBuffer(length) {

lib/fs.js

@@ -3,7 +3,6 @@
 
 'use strict';
 
-const SlowBuffer = require('buffer').SlowBuffer;
 const util = require('util');
 const pathModule = require('path');
 
@@ -321,7 +320,7 @@ ReadFileContext.prototype.read = function() {
   var length;
 
   if (this.size === 0) {
-    buffer = this.buffer = SlowBuffer(kReadFileBufferLength);
+    buffer = this.buffer = Buffer.allocUnsafeSlow(kReadFileBufferLength);
     offset = 0;
     length = kReadFileBufferLength;
   } else {
@@ -389,7 +388,7 @@ function readFileAfterStat(err, st) {
     return context.close(err);
   }
 
-  context.buffer = SlowBuffer(size);
+  context.buffer = Buffer.allocUnsafeSlow(size);
   context.read();
 }
 

test/parallel/test-buffer-alloc.js

@@ -3,7 +3,6 @@ var common = require('../common');
 var assert = require('assert');
 
 var Buffer = require('buffer').Buffer;
-var SlowBuffer = require('buffer').SlowBuffer;
 
 // counter to ensure unique value is always copied
 var cntr = 0;
@@ -428,7 +427,7 @@ for (let i = 0; i < Buffer.byteLength(utf8String); i++) {
 
 {
   // also from a non-pooled instance
-  const b = new SlowBuffer(5);
+  const b = Buffer.allocUnsafeSlow(5);
   const c = b.slice(0, 4);
   const d = c.slice(0, 2);
   assert.equal(c.parent, d.parent);
@@ -1305,7 +1304,7 @@ assert.throws(function() {
 
   // try to slice a zero length Buffer
   // see https://github.com/joyent/node/issues/5881
-  SlowBuffer(0).slice(0, 1);
+  Buffer.alloc(0).slice(0, 1);
 })();
 
 // Regression test for #5482: should throw but not assert in C++ land.
@@ -1336,7 +1335,7 @@ assert.throws(function() {
 }, RangeError);
 
 assert.throws(function() {
-  SlowBuffer((-1 >>> 0) + 1);
+  Buffer.allocUnsafeSlow((-1 >>> 0) + 1);
 }, RangeError);
 
 if (common.hasCrypto) {
@@ -1435,14 +1434,13 @@ assert.throws(function() {
 }, regErrorMsg);
 
 
-// Test prototype getters don't throw
-assert.equal(Buffer.prototype.parent, undefined);
-assert.equal(Buffer.prototype.offset, undefined);
-assert.equal(SlowBuffer.prototype.parent, undefined);
-assert.equal(SlowBuffer.prototype.offset, undefined);
-
-
 // Test that ParseArrayIndex handles full uint32
 assert.throws(function() {
   Buffer.from(new ArrayBuffer(0), -1 >>> 0);
 }, /RangeError: 'offset' is out of bounds/);
+
+// Unpooled buffer (replaces SlowBuffer)
+const ubuf = Buffer.allocUnsafeSlow(10);
+assert(ubuf);
+assert(ubuf.buffer);
+assert.equal(ubuf.buffer.byteLength, 10);