Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: plug memory leaks #2352

Merged
merged 4 commits into from
Aug 13, 2015
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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 <fedor@indutny.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
  • Loading branch information
bnoordhuis committed Aug 13, 2015
commit 64577463dfbc7ebe646c6c5a06e4b309b2757205
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 @@ -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);
}

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

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

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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, this one takes the ownership of base. You should not really fix it.

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