src: plug memory leaks

bnoordhuis · bnoordhuis · commit 64577463dfbc · 2015-08-13T20:03:41.000+02:00
In a few places dynamic memory was passed to the Buffer::New() overload that makes a copy of the input, not the one that takes ownership. This commit is a band-aid to fix the memory leaks. Longer term, we should look into using C++11 move semantics more effectively. Fixes: nodejs#2308 PR-URL: nodejs#2352 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
diff --git a/src/node_buffer.h b/src/node_buffer.h
@@ -67,12 +67,17 @@ static inline bool IsWithinBounds(size_t off, size_t len, size_t max) {
 // in src/node_internals.h because of a circular dependency.
 #if defined(NODE_WANT_INTERNALS)
 v8::MaybeLocal<v8::Object> New(Environment* env, size_t size);
+// Makes a copy of |data|.
 v8::MaybeLocal<v8::Object> New(Environment* env, const char* data, size_t len);
+// Takes ownership of |data|.
 v8::MaybeLocal<v8::Object> New(Environment* env,
                                char* data,
                                size_t length,
                                FreeCallback callback,
                                void* hint);
+// Takes ownership of |data|.  Must allocate |data| with malloc() or realloc()
+// because ArrayBufferAllocator::Free() deallocates it again with free().
+// Mixing operator new and free() is undefined behavior so don't do that.
 v8::MaybeLocal<v8::Object> Use(Environment* env, char* data, size_t length);
 #endif  // defined(NODE_WANT_INTERNALS)
 
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -4479,7 +4479,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
     return env->ThrowError("Failed to compute ECDH key");
   }
 
-  Local<Object> buf = Buffer::New(env, out, out_len).ToLocalChecked();
+  Local<Object> buf = Buffer::Use(env, out, out_len).ToLocalChecked();
   args.GetReturnValue().Set(buf);
 }
 
@@ -4544,7 +4544,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
   }
 
   Local<Object> buf =
-      Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
+      Buffer::Use(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
   args.GetReturnValue().Set(buf);
 }
 
@@ -4947,7 +4947,7 @@ void RandomBytesCheck(RandomBytesRequest* req, Local<Value> argv[2]) {
     size_t size;
     req->return_memory(&data, &size);
     argv[0] = Null(req->env()->isolate());
-    argv[1] = Buffer::New(req->env(), data, size).ToLocalChecked();
+    argv[1] = Buffer::Use(req->env(), data, size).ToLocalChecked();
   }
 }
 
diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc
@@ -223,7 +223,7 @@ void StreamWrap::OnReadImpl(ssize_t nread,
     CHECK_EQ(pending, UV_UNKNOWN_HANDLE);
   }
 
-  Local<Object> obj = Buffer::New(env, base, nread).ToLocalChecked();
+  Local<Object> obj = Buffer::Use(env, base, nread).ToLocalChecked();
   wrap->EmitData(nread, obj, pending_obj);
 }
 
diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc
@@ -408,7 +408,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle,
   }
 
   char* base = static_cast<char*>(realloc(buf->base, nread));
-  argv[2] = Buffer::New(env, base, nread).ToLocalChecked();
+  argv[2] = Buffer::Use(env, base, nread).ToLocalChecked();
   argv[3] = AddressToJS(env, addr);
   wrap->MakeCallback(env->onmessage_string(), ARRAY_SIZE(argv), argv);
 }

Original file line number	Diff line number	Diff line change
`@@ -4479,7 +4479,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {`
`4479`	`4479`	`return env->ThrowError("Failed to compute ECDH key");`
`4480`	`4480`	`}`
`4481`	`4481`
`4482`		`- Local<Object> buf = Buffer::New(env, out, out_len).ToLocalChecked();`
	`4482`	`+ Local<Object> buf = Buffer::Use(env, out, out_len).ToLocalChecked();`
`4483`	`4483`	`args.GetReturnValue().Set(buf);`
`4484`	`4484`	`}`
`4485`	`4485`
`@@ -4544,7 +4544,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {`
`4544`	`4544`	`}`
`4545`	`4545`
`4546`	`4546`	`Local<Object> buf =`
`4547`		`- Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();`
	`4547`	`+ Buffer::Use(env, reinterpret_cast<char*>(out), size).ToLocalChecked();`
`4548`	`4548`	`args.GetReturnValue().Set(buf);`
`4549`	`4549`	`}`
`4550`	`4550`
`@@ -4947,7 +4947,7 @@ void RandomBytesCheck(RandomBytesRequest* req, Local<Value> argv[2]) {`
`4947`	`4947`	`size_t size;`
`4948`	`4948`	`req->return_memory(&data, &size);`
`4949`	`4949`	`argv[0] = Null(req->env()->isolate());`
`4950`		`- argv[1] = Buffer::New(req->env(), data, size).ToLocalChecked();`
	`4950`	`+ argv[1] = Buffer::Use(req->env(), data, size).ToLocalChecked();`
`4951`	`4951`	`}`
`4952`	`4952`	`}`
`4953`	`4953`
Original file line number	Diff line number	Diff line change
`@@ -223,7 +223,7 @@ void StreamWrap::OnReadImpl(ssize_t nread,`
`223`	`223`	`CHECK_EQ(pending, UV_UNKNOWN_HANDLE);`
`224`	`224`	`}`
`225`	`225`
`226`		`- Local<Object> obj = Buffer::New(env, base, nread).ToLocalChecked();`
	`226`	`+ Local<Object> obj = Buffer::Use(env, base, nread).ToLocalChecked();`
`227`	`227`	`wrap->EmitData(nread, obj, pending_obj);`
`228`	`228`	`}`
`229`	`229`
Original file line number	Diff line number	Diff line change
`@@ -408,7 +408,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle,`
`408`	`408`	`}`
`409`	`409`
`410`	`410`	`char* base = static_cast<char*>(realloc(buf->base, nread));`
`411`		`- argv[2] = Buffer::New(env, base, nread).ToLocalChecked();`
	`411`	`+ argv[2] = Buffer::Use(env, base, nread).ToLocalChecked();`
`412`	`412`	`argv[3] = AddressToJS(env, addr);`
`413`	`413`	`wrap->MakeCallback(env->onmessage_string(), ARRAY_SIZE(argv), argv);`
`414`	`414`	`}`