[v6/v7] Double free on garbage collection when allocating inside libuv callback

[Dagrut]

• Node.js Version: 6.10.3-1nodesource1~jessie1 and 7.10.0-1nodesource1~jessie1 (debian packages)
• OS: (uname -a : ) Linux unicorn 3.16.0-4-amd64 Update README for help #1 SMP Debian 3.16.43-2 (2017-04-30) x86_64 GNU/Linux
• Scope (install, code, runtime, meta, other?): native module
• Module (and version) (if relevant): [email protected]

I encountered an error (described here : binarysec/node-tuntap#6 ) in which there seems to be a double free() error. It happens in the void Tuntap::do_read() function of https://github.com/binarysec/node-tuntap/blob/master/src/tuntap.cc#L621 . As a short version, if I do

void Tuntap::do_read() {
	Isolate* isolate = Isolate::GetCurrent();
	HandleScope scope(isolate);
	
	Local<Object> ret_buff;
}

It does not crash, but if I do :

void Tuntap::do_read() {
	Isolate* isolate = Isolate::GetCurrent();
	HandleScope scope(isolate);
	
	Local<Object> ret_buff;
	ret_buff = node::Buffer::New(isolate, (char*) this->read_buff, ret).ToLocalChecked();
}

It crashes after the third call & garbage collection.

This function is called from a libuv event callback, which does not provide any isolate or else, so I have to use GetCurrent() (even if I also do it everywhere else, but I'll change that).

Am I doing something wrong, or is there a bug inside node/v8 ?

[bnoordhuis]

That code should probably be using Buffer::Copy(). Buffer::New() takes ownership of the pointer.

EDIT: More specifically, the three argument overload of Buffer::New() does.

[Dagrut]

Wow, thanks! That was a fast answer! 😄
Indeed, using Copy instead of New fixed the issue! I may have to fix other projects, then... 😆