buffer: toArrayBuffer() memory leak

[trevnorris]

Simple test script:

var ITER = 1e6;

var b = new Buffer(1024 * 1024);

for (var i = 0; i < ITER; i++) {
  b.toArrayBuffer();
}

This would cause my machine to swap to death if I hadn't set application memory consumption limits.

[tjfontaine]

I'm not sure there's much we can do, it's a tight loop allocating a crap load of memory, give the gc some time to run.

[trevnorris]

@tjfontaine rewrite here:

var ITER = 1e3;
var b = new Buffer(1024 * 1024);

if (!global.gc)
  throw new Error('Run with --expose-gc');

for (var i = 0; i < ITER; i++) {
  b.toArrayBuffer();
  gc();
}

Running:

$ /usr/bin/time ./node --expose-gc test.js
1.14user 0.28system 0:01.32elapsed 108%CPU (0avgtext+0avgdata 1040668maxresident)k
0inputs+0outputs (0major+260865minor)pagefaults 0swaps

Evidence right now is that there's a leak. Going to take a closer look.

[trevnorris]

Issue is that ArrayBuffer::New(Isolate*, void*, size_t) treats the ArrayBuffer as "Externalized". Meaning v8 GC doesn't do the cleanup. Fixing this now.

[trevnorris]

I've posted a question on the v8-users mailing list.

As of right now I don't see how doing this is possible, and this presents a fairly significant memory leak. For the moment I think it smart to remove Buffer#toArrayBuffer() until the issue can be resolved.

/cc @tjfontaine

[trevnorris]

Unfortunately the quickest/safest fix was to remove Buffer#toArrayBuffer() for the time being: b1a44df

This is still an API we'd be fine to support, but need to work with the v8 team before it can happen reliably. For that reason, going to leave this issue open.

[xdenser]

Maybe some wrapper will help?
i.e. in pseudo code:
wrapperObject.proto == externalizedArrayBuffer
wrapperObject.onDestroy == externalizedArrayBuffer.free

[vkurchatkin]

@trevnorris here is quite simple solution:

Buffer.prototype.toArrayBuffer = function() {
  var arrayBuffer = new ArrayBuffer(this.length);
  var array = new Uint8Array(arrayBuffer);

  smalloc.copyOnto(this, 0, array, 0, this.length);

  return arrayBuffer;
};

[trevnorris]

@vkurchatkin WTF... That's a really strange side effect of V8's implementation details. It shouldn't work to use v8::Object::GetIndexedPropertiesExternalArrayData() from a v8::TypedArray. Instead the user should need to use v8::ArrayBuffer::Contents.

Thanks for the find. We'll have to confirm w/ the V8 team there aren't any strange unforeseen issues before using this solution.

UPDATE: Waiting for a response from the following: https://groups.google.com/d/msg/v8-dev/qathcB2-M-A/MVKwE8oHVH4J

[jasnell]

@trevnorris ... any updates on this one?

[trevnorris]

@jasnell Never got a response. Starting in 4.3 Buffers are backed by Uint8Array anyway so it's a non-issue down the pipe. I don't think it's worth the time to reimplement this feature before then.

Anway, technically this was fixed when the commit was reverted.