Skip to content

Commit bff9bcd

Browse files
bnoordhuisrvagg
authored andcommitted
src: plug memory leaks
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: #2308 PR-URL: #2352 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
1 parent 1f12e03 commit bff9bcd

File tree

4 files changed

+10
-5
lines changed

4 files changed

+10
-5
lines changed

src/node_buffer.h

+5
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,17 @@ static inline bool IsWithinBounds(size_t off, size_t len, size_t max) {
6767
// in src/node_internals.h because of a circular dependency.
6868
#if defined(NODE_WANT_INTERNALS)
6969
v8::MaybeLocal<v8::Object> New(Environment* env, size_t size);
70+
// Makes a copy of |data|.
7071
v8::MaybeLocal<v8::Object> New(Environment* env, const char* data, size_t len);
72+
// Takes ownership of |data|.
7173
v8::MaybeLocal<v8::Object> New(Environment* env,
7274
char* data,
7375
size_t length,
7476
FreeCallback callback,
7577
void* hint);
78+
// Takes ownership of |data|. Must allocate |data| with malloc() or realloc()
79+
// because ArrayBufferAllocator::Free() deallocates it again with free().
80+
// Mixing operator new and free() is undefined behavior so don't do that.
7681
v8::MaybeLocal<v8::Object> Use(Environment* env, char* data, size_t length);
7782
#endif // defined(NODE_WANT_INTERNALS)
7883

src/node_crypto.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -4477,7 +4477,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo<Value>& args) {
44774477
return env->ThrowError("Failed to compute ECDH key");
44784478
}
44794479

4480-
Local<Object> buf = Buffer::New(env, out, out_len).ToLocalChecked();
4480+
Local<Object> buf = Buffer::Use(env, out, out_len).ToLocalChecked();
44814481
args.GetReturnValue().Set(buf);
44824482
}
44834483

@@ -4542,7 +4542,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo<Value>& args) {
45424542
}
45434543

45444544
Local<Object> buf =
4545-
Buffer::New(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
4545+
Buffer::Use(env, reinterpret_cast<char*>(out), size).ToLocalChecked();
45464546
args.GetReturnValue().Set(buf);
45474547
}
45484548

@@ -4945,7 +4945,7 @@ void RandomBytesCheck(RandomBytesRequest* req, Local<Value> argv[2]) {
49454945
size_t size;
49464946
req->return_memory(&data, &size);
49474947
argv[0] = Null(req->env()->isolate());
4948-
argv[1] = Buffer::New(req->env(), data, size).ToLocalChecked();
4948+
argv[1] = Buffer::Use(req->env(), data, size).ToLocalChecked();
49494949
}
49504950
}
49514951

src/stream_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ void StreamWrap::OnReadImpl(ssize_t nread,
223223
CHECK_EQ(pending, UV_UNKNOWN_HANDLE);
224224
}
225225

226-
Local<Object> obj = Buffer::New(env, base, nread).ToLocalChecked();
226+
Local<Object> obj = Buffer::Use(env, base, nread).ToLocalChecked();
227227
wrap->EmitData(nread, obj, pending_obj);
228228
}
229229

src/udp_wrap.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle,
408408
}
409409

410410
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();
412412
argv[3] = AddressToJS(env, addr);
413413
wrap->MakeCallback(env->onmessage_string(), ARRAY_SIZE(argv), argv);
414414
}

0 commit comments

Comments
 (0)