This repository has been archived by the owner. It is now read-only.
Permalink
Browse files

src: add HandleScope in HandleWrap::OnClose

Fixes a 4 byte leak on handles closing. AKA The Walmart leak.

MakeCallback doesn't have a HandleScope. That means the callers scope
will retain ownership of created handles from MakeCallback and related.
There is by default a wrapping HandleScope before uv_run, if the caller
doesn't have a HandleScope on the stack the global will take ownership
which won't be reaped until the uv loop exits.

If a uv callback is fired, and there is no enclosing HandleScope in the
cb, you will appear to leak 4-bytes for every invocation. Take heed.

cc @hueniverse
  • Loading branch information...
tjfontaine committed Nov 12, 2013
1 parent ac799ba commit 16934d9210546bf19d4af8d98652aa5d636ce693
Showing with 13 additions and 0 deletions.
  1. +2 −0 src/handle_wrap.cc
  2. +11 −0 src/node.h
@@ -134,6 +134,8 @@ HandleWrap::~HandleWrap() {


void HandleWrap::OnClose(uv_handle_t* handle) {
HandleScope scope;

HandleWrap* wrap = static_cast<HandleWrap*>(handle->data);

// The wrap object should still be there.
@@ -238,6 +238,17 @@ node_module_struct* get_builtin_module(const char *name);
*/
NODE_EXTERN void AtExit(void (*cb)(void* arg), void* arg = 0);

/*
* MakeCallback doesn't have a HandleScope. That means the callers scope
* will retain ownership of created handles from MakeCallback and related.
* There is by default a wrapping HandleScope before uv_run, if the caller
* doesn't have a HandleScope on the stack the global will take ownership
* which won't be reaped until the uv loop exits.
*
* If a uv callback is fired, and there is no enclosing HandleScope in the
* cb, you will appear to leak 4-bytes for every invocation. Take heed.
*/

NODE_EXTERN void SetErrno(uv_err_t err);
NODE_EXTERN v8::Handle<v8::Value>
MakeCallback(const v8::Handle<v8::Object> object,

4 comments on commit 16934d9

@ronkorving

This comment has been minimized.

Copy link

ronkorving replied Jan 7, 2014

Did this leak affect Node 0.8, and if it did, has it been backported?

@indutny

This comment has been minimized.

Copy link
Member

indutny replied Jan 7, 2014

Nope it doesn't affect it.

@ronkorving

This comment has been minimized.

Copy link

ronkorving replied Jan 7, 2014

Thanks

@doron2402

This comment has been minimized.

Copy link

doron2402 replied Jan 26, 2016

👍

Please sign in to comment.