From 9d4624a39935200bc284ffbbd033e233f6b844bb Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 12 Oct 2015 16:08:13 -0400 Subject: [PATCH 1/2] buffer: fix assertion error in WeakCallback `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: https://github.com/nodejs/node/pull/3080#issuecomment-147502167 --- src/node_buffer.cc | 10 ++++++---- test/addons/buffer-free-callback/test.js | 4 ++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 4df7a9f1b4d065..be790cb9f8e3bf 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -148,11 +148,13 @@ void CallbackInfo::WeakCallback( void CallbackInfo::WeakCallback(Isolate* isolate, Local object) { - SPREAD_ARG(object, obj); - CHECK_EQ(obj_offset, 0); - CHECK_EQ(obj_c.ByteLength(), obj_length); + CHECK(object->IsArrayBuffer()); + Local buf = object.As(); + ArrayBuffer::Contents obj_c = buf->GetContents(); + char* const obj_data = static_cast(obj_c.Data()); + CHECK_NE(obj_data, nullptr); - obj->Buffer()->Neuter(); + buf->Neuter(); callback_(obj_data, hint_); int64_t change_in_bytes = -static_cast(sizeof(*this)); isolate->AdjustAmountOfExternalAllocatedMemory(change_in_bytes); diff --git a/test/addons/buffer-free-callback/test.js b/test/addons/buffer-free-callback/test.js index b95f171de2467e..6b3ff1f6d97e2d 100644 --- a/test/addons/buffer-free-callback/test.js +++ b/test/addons/buffer-free-callback/test.js @@ -8,3 +8,7 @@ var buf = binding.alloc(); var slice = buf.slice(32); buf = null; binding.check(slice); +slice = null; +gc(); +gc(); +gc(); From a0569882c813e6910b92c4584a2dd6811f21809e Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 12 Oct 2015 16:18:10 -0400 Subject: [PATCH 2/2] ... --- src/node_buffer.cc | 3 ++- test/addons/buffer-free-callback/binding.cc | 2 +- test/addons/buffer-free-callback/test.js | 25 ++++++++++++++------- 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index be790cb9f8e3bf..a472d0c95e9225 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -152,7 +152,8 @@ void CallbackInfo::WeakCallback(Isolate* isolate, Local object) { Local buf = object.As(); ArrayBuffer::Contents obj_c = buf->GetContents(); char* const obj_data = static_cast(obj_c.Data()); - CHECK_NE(obj_data, nullptr); + if (buf->ByteLength() != 0) + CHECK_NE(obj_data, nullptr); buf->Neuter(); callback_(obj_data, hint_); diff --git a/test/addons/buffer-free-callback/binding.cc b/test/addons/buffer-free-callback/binding.cc index a1e7773e95103d..e5298a0063e5fa 100644 --- a/test/addons/buffer-free-callback/binding.cc +++ b/test/addons/buffer-free-callback/binding.cc @@ -16,7 +16,7 @@ void Alloc(const v8::FunctionCallbackInfo& args) { args.GetReturnValue().Set(node::Buffer::New( isolate, buf, - sizeof(buf), + args[0]->IntegerValue(), FreeCallback, nullptr).ToLocalChecked()); } diff --git a/test/addons/buffer-free-callback/test.js b/test/addons/buffer-free-callback/test.js index 6b3ff1f6d97e2d..6ee328d5222049 100644 --- a/test/addons/buffer-free-callback/test.js +++ b/test/addons/buffer-free-callback/test.js @@ -4,11 +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); -slice = null; -gc(); -gc(); -gc(); + +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);