Skip to content

Commit

Permalink
buffer: fix assertion error in WeakCallback
Browse files Browse the repository at this point in the history
`CallbackInfo` is now bound to `ArrayBuffer` instance, not `Uint8Array`,
therefore `SPREAD_ARG` will abort with:

    Assertion failed: ((object)->IsUint8Array())

Make changes necessary to migrate it to `ArrayBuffer`.

See: #3080 (comment)

Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
PR-URL: #3329
  • Loading branch information
indutny authored and jasnell committed Oct 13, 2015
1 parent 7d94611 commit 931118c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 10 deletions.
13 changes: 8 additions & 5 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,14 @@ void CallbackInfo::WeakCallback(


void CallbackInfo::WeakCallback(Isolate* isolate, Local<Object> object) {
SPREAD_ARG(object, obj);
CHECK_EQ(obj_offset, 0);
CHECK_EQ(obj_c.ByteLength(), obj_length);

obj->Buffer()->Neuter();
CHECK(object->IsArrayBuffer());
Local<ArrayBuffer> buf = object.As<ArrayBuffer>();
ArrayBuffer::Contents obj_c = buf->GetContents();
char* const obj_data = static_cast<char*>(obj_c.Data());
if (buf->ByteLength() != 0)
CHECK_NE(obj_data, nullptr);

buf->Neuter();
callback_(obj_data, hint_);
int64_t change_in_bytes = -static_cast<int64_t>(sizeof(*this));
isolate->AdjustAmountOfExternalAllocatedMemory(change_in_bytes);
Expand Down
2 changes: 1 addition & 1 deletion test/addons/buffer-free-callback/binding.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ void Alloc(const v8::FunctionCallbackInfo<v8::Value>& args) {
args.GetReturnValue().Set(node::Buffer::New(
isolate,
buf,
sizeof(buf),
args[0]->IntegerValue(),
FreeCallback,
nullptr).ToLocalChecked());
}
Expand Down
21 changes: 17 additions & 4 deletions test/addons/buffer-free-callback/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,20 @@
require('../../common');
var assert = require('assert');
var binding = require('./build/Release/binding');
var buf = binding.alloc();
var slice = buf.slice(32);
buf = null;
binding.check(slice);

function check(size) {
var buf = binding.alloc(size);
var slice = buf.slice(size >>> 1);

buf = null;
binding.check(slice);
slice = null;
gc();
gc();
gc();
}

check(64);

// Empty ArrayBuffer does not allocate data, worth checking
check(0);

0 comments on commit 931118c

Please sign in to comment.