Skip to content

Commit

Permalink
stream_wrap: add HandleScope's in uv callbacks
Browse files Browse the repository at this point in the history
Ensure that no handles will leak into global HandleScope by adding
HandleScope's in all JS-calling libuv callbacks in `stream_wrap.cc`.

Fix: #1075
PR-URL: #1078
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
indutny committed Mar 6, 2015
1 parent b27931b commit 583a868
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 14 deletions.
8 changes: 8 additions & 0 deletions src/stream_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,9 @@ void StreamWrap::OnAlloc(uv_handle_t* handle,
size_t suggested_size,
uv_buf_t* buf) {
StreamWrap* wrap = static_cast<StreamWrap*>(handle->data);
HandleScope scope(wrap->env()->isolate());
Context::Scope context_scope(wrap->env()->context());

CHECK_EQ(wrap->stream(), reinterpret_cast<uv_stream_t*>(handle));

return static_cast<StreamBase*>(wrap)->OnAlloc(suggested_size, buf);
Expand Down Expand Up @@ -229,6 +232,7 @@ void StreamWrap::OnReadCommon(uv_stream_t* handle,
const uv_buf_t* buf,
uv_handle_type pending) {
StreamWrap* wrap = static_cast<StreamWrap*>(handle->data);
HandleScope scope(wrap->env()->isolate());

This comment has been minimized.

Copy link
@bnoordhuis

bnoordhuis Mar 6, 2015

Member

Shouldn't this also have a Context::Scope?

This comment has been minimized.

Copy link
@indutny

indutny Mar 6, 2015

Author Member

Gosh, I totally missed it. Mind submitting a PR?

This comment has been minimized.

Copy link
@bnoordhuis

bnoordhuis Mar 6, 2015

Member

// We should not be getting this callback if someone as already called
// uv_close() on the handle.
Expand Down Expand Up @@ -280,6 +284,8 @@ int StreamWrap::DoShutdown(ShutdownWrap* req_wrap) {

void StreamWrap::AfterShutdown(uv_shutdown_t* req, int status) {
ShutdownWrap* req_wrap = ContainerOf(&ShutdownWrap::req_, req);
HandleScope scope(req_wrap->env()->isolate());
Context::Scope context_scope(req_wrap->env()->context());
req_wrap->Done(status);
}

Expand Down Expand Up @@ -354,6 +360,8 @@ int StreamWrap::DoWrite(WriteWrap* w,

void StreamWrap::AfterWrite(uv_write_t* req, int status) {
WriteWrap* req_wrap = ContainerOf(&WriteWrap::req_, req);
HandleScope scope(req_wrap->env()->isolate());
Context::Scope context_scope(req_wrap->env()->context());
req_wrap->Done(status);
}

Expand Down
14 changes: 0 additions & 14 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ using v8::Function;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::Handle;
using v8::HandleScope;
using v8::Integer;
using v8::Local;
using v8::Null;
Expand Down Expand Up @@ -251,8 +250,6 @@ void TLSWrap::SSLInfoCallback(const SSL* ssl_, int where, int ret) {
SSL* ssl = const_cast<SSL*>(ssl_);
TLSWrap* c = static_cast<TLSWrap*>(SSL_get_app_data(ssl));
Environment* env = c->env();
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
Local<Object> object = c->object();

if (where & SSL_CB_HANDSHAKE_START) {
Expand Down Expand Up @@ -395,9 +392,6 @@ void TLSWrap::ClearOut() {
if (eof_)
return;

HandleScope handle_scope(env()->isolate());
Context::Scope context_scope(env()->context());

CHECK_NE(ssl_, nullptr);

char out[kClearOutChunkSize];
Expand Down Expand Up @@ -470,9 +464,6 @@ bool TLSWrap::ClearIn() {
return true;
}

HandleScope handle_scope(env()->isolate());
Context::Scope context_scope(env()->context());

// Error or partial write
int err;
Local<Value> arg = GetSSLError(written, &err, &error_);
Expand Down Expand Up @@ -588,8 +579,6 @@ int TLSWrap::DoWrite(WriteWrap* w,

if (i != count) {
int err;
HandleScope handle_scope(env()->isolate());
Context::Scope context_scope(env()->context());
Local<Value> arg = GetSSLError(written, &err, &error_);
if (!arg.IsEmpty())
return UV_EPROTO;
Expand Down Expand Up @@ -662,8 +651,6 @@ void TLSWrap::DoRead(ssize_t nread,
eof_ = true;
}

HandleScope handle_scope(env()->isolate());
Context::Scope context_scope(env()->context());
OnRead(nread, nullptr);
return;
}
Expand Down Expand Up @@ -796,7 +783,6 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg) {
if (servername == nullptr)
return SSL_TLSEXT_ERR_OK;

HandleScope scope(env->isolate());
// Call the SNI callback and use its return value as context
Local<Object> object = p->object();
Local<Value> ctx = object->Get(env->sni_context_string());
Expand Down

0 comments on commit 583a868

Please sign in to comment.