Skip to content

Commit

Permalink
buffer: FreeCallback should be tied to ArrayBuffer
Browse files Browse the repository at this point in the history
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: #3198
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
indutny committed Oct 6, 2015
1 parent 7d66749 commit 9d4063e
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ MaybeLocal<Object> New(Environment* env,
if (!mb.FromMaybe(false))
return Local<Object>();

CallbackInfo::New(env->isolate(), ui, callback, hint);
CallbackInfo::New(env->isolate(), ab, callback, hint);
return scope.Escape(ui);
}

Expand Down
36 changes: 36 additions & 0 deletions test/addons/buffer-free-callback/binding.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#include <node.h>
#include <node_buffer.h>
#include <util.h>
#include <v8.h>

static int alive;
static char buf[1024];

static void FreeCallback(char* data, void* hint) {
alive--;
}

void Alloc(const v8::FunctionCallbackInfo<v8::Value>& 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<v8::Value>& args) {
v8::Isolate* isolate = args.GetIsolate();
isolate->RequestGarbageCollectionForTesting(
v8::Isolate::kFullGarbageCollection);
CHECK_GT(alive, 0);
}

void init(v8::Local<v8::Object> target) {
NODE_SET_METHOD(target, "alloc", Alloc);
NODE_SET_METHOD(target, "check", Check);
}

NODE_MODULE(binding, init);
8 changes: 8 additions & 0 deletions test/addons/buffer-free-callback/binding.gyp
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
'targets': [
{
'target_name': 'binding',
'sources': [ 'binding.cc' ]
}
]
}
9 changes: 9 additions & 0 deletions test/addons/buffer-free-callback/test.js
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit 9d4063e

Please sign in to comment.