Skip to content

Commit

Permalink
buffer: add Buffer.allocUnsafeSlow(size)
Browse files Browse the repository at this point in the history
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: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
  • Loading branch information
jasnell committed Apr 15, 2016
1 parent b488b19 commit 6275249
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 40 deletions.
8 changes: 8 additions & 0 deletions benchmark/buffers/buffer-creation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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++) {
Expand Down
94 changes: 72 additions & 22 deletions doc/api/buffer.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
29 changes: 25 additions & 4 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
5 changes: 2 additions & 3 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

'use strict';

const SlowBuffer = require('buffer').SlowBuffer;
const util = require('util');
const pathModule = require('path');

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -389,7 +388,7 @@ function readFileAfterStat(err, st) {
return context.close(err);
}

context.buffer = SlowBuffer(size);
context.buffer = Buffer.allocUnsafeSlow(size);
context.read();
}

Expand Down
20 changes: 9 additions & 11 deletions test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -1336,7 +1335,7 @@ assert.throws(function() {
}, RangeError);

assert.throws(function() {
SlowBuffer((-1 >>> 0) + 1);
Buffer.allocUnsafeSlow((-1 >>> 0) + 1);
}, RangeError);

if (common.hasCrypto) {
Expand Down Expand Up @@ -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);

0 comments on commit 6275249

Please sign in to comment.