From 9d4063e1df3d9239d66f3332059a89c5944c7e57 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Mon, 5 Oct 2015 23:26:13 -0400 Subject: [PATCH] buffer: FreeCallback should be tied to ArrayBuffer FreeCallback should be invoked on the storage disposal (`ArrayBuffer`), not when the view (`Uint8Array` or `Buffer`) is disposed. This causes bug and crashes in addons which create buffers and store only slices of them. PR-URL: https://github.com/nodejs/node/pull/3198 Reviewed-By: Ben Noordhuis Reviewed-By: Trevor Norris --- src/node_buffer.cc | 2 +- test/addons/buffer-free-callback/binding.cc | 36 ++++++++++++++++++++ test/addons/buffer-free-callback/binding.gyp | 8 +++++ test/addons/buffer-free-callback/test.js | 9 +++++ 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 test/addons/buffer-free-callback/binding.cc create mode 100644 test/addons/buffer-free-callback/binding.gyp create mode 100644 test/addons/buffer-free-callback/test.js diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 5e0de52322496b..b6ce6bc417c941 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -356,7 +356,7 @@ MaybeLocal New(Environment* env, if (!mb.FromMaybe(false)) return Local(); - CallbackInfo::New(env->isolate(), ui, callback, hint); + CallbackInfo::New(env->isolate(), ab, callback, hint); return scope.Escape(ui); } diff --git a/test/addons/buffer-free-callback/binding.cc b/test/addons/buffer-free-callback/binding.cc new file mode 100644 index 00000000000000..a1e7773e95103d --- /dev/null +++ b/test/addons/buffer-free-callback/binding.cc @@ -0,0 +1,36 @@ +#include +#include +#include +#include + +static int alive; +static char buf[1024]; + +static void FreeCallback(char* data, void* hint) { + alive--; +} + +void Alloc(const v8::FunctionCallbackInfo& args) { + v8::Isolate* isolate = args.GetIsolate(); + alive++; + args.GetReturnValue().Set(node::Buffer::New( + isolate, + buf, + sizeof(buf), + FreeCallback, + nullptr).ToLocalChecked()); +} + +void Check(const v8::FunctionCallbackInfo& args) { + v8::Isolate* isolate = args.GetIsolate(); + isolate->RequestGarbageCollectionForTesting( + v8::Isolate::kFullGarbageCollection); + CHECK_GT(alive, 0); +} + +void init(v8::Local target) { + NODE_SET_METHOD(target, "alloc", Alloc); + NODE_SET_METHOD(target, "check", Check); +} + +NODE_MODULE(binding, init); diff --git a/test/addons/buffer-free-callback/binding.gyp b/test/addons/buffer-free-callback/binding.gyp new file mode 100644 index 00000000000000..3bfb84493f3e87 --- /dev/null +++ b/test/addons/buffer-free-callback/binding.gyp @@ -0,0 +1,8 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.cc' ] + } + ] +} diff --git a/test/addons/buffer-free-callback/test.js b/test/addons/buffer-free-callback/test.js new file mode 100644 index 00000000000000..14bc350738d68c --- /dev/null +++ b/test/addons/buffer-free-callback/test.js @@ -0,0 +1,9 @@ +'use strict'; +// Flags: --expose-gc + +var assert = require('assert'); +var binding = require('./build/Release/binding'); +var buf = binding.alloc(); +var slice = buf.slice(32); +buf = null; +binding.check(slice);