Skip to content

Commit

Permalink
src: plug memory leaks
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
bnoordhuis authored and rvagg committed Aug 17, 2015
1 parent 1f12e03 commit bff9bcd
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 5 deletions.
5 changes: 5 additions & 0 deletions src/node_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
6 changes: 3 additions & 3 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4477,7 +4477,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);
}

Expand Down Expand Up @@ -4542,7 +4542,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);
}

Expand Down Expand Up @@ -4945,7 +4945,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();
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/stream_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/udp_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit bff9bcd

Please sign in to comment.