From 2bedf6efb16deed2e5693c38fde75611826552aa Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 19 Jul 2013 22:35:23 +0200 Subject: [PATCH] src: fix persistent handle lifecycle issue Commit 636ca7c adds an optimization that casts strong Persistent handles directly to Local handles to avoid the overhead of creating new HandleScope-rooted Local handles all the time. One gotcha that I missed is that it's no longer legal to reference the Local after calling Persistent::Dispose(). This commit addresses that. --- src/cares_wrap.cc | 4 ++-- src/node_crypto.cc | 2 +- src/node_internals.h | 6 ++++++ src/node_script.cc | 4 ++-- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index 2617ce5097d..4a55453f697 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -744,7 +744,7 @@ static void Query(const FunctionCallbackInfo& args) { // We must cache the wrap's js object here, because cares might make the // callback from the wrap->Send stack. This will destroy the wrap's internal // object reference, causing wrap->object() to return an empty handle. - Local object = wrap->object(); + Local object = Local::New(node_isolate, wrap->persistent()); String::Utf8Value name(args[0]); @@ -772,7 +772,7 @@ static void QueryWithFamily(const FunctionCallbackInfo& args) { // We must cache the wrap's js object here, because cares might make the // callback from the wrap->Send stack. This will destroy the wrap's internal // object reference, causing wrap->object() to return an empty handle. - Local object = wrap->object(); + Local object = Local::New(node_isolate, wrap->persistent()); String::Utf8Value name(args[0]); int family = args[1]->Int32Value(); diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 0ecf14ef8a9..8530cb51c99 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -3248,7 +3248,7 @@ void EIO_PBKDF2After(uv_work_t* work_req, int status) { assert(status == 0); pbkdf2_req* req = container_of(work_req, pbkdf2_req, work_req); HandleScope scope(node_isolate); - Local obj = PersistentToLocal(req->obj); + Local obj = Local::New(node_isolate, req->obj); req->obj.Dispose(); Local argv[2]; EIO_PBKDF2After(req, argv); diff --git a/src/node_internals.h b/src/node_internals.h index d59cd2e5d22..92c4fbd213f 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -62,10 +62,16 @@ public: void operator=(v8::Handle that); }; +// If persistent.IsWeak() == false, then do not call persistent.Dispose() +// while the returned Local is still in scope, it will destroy the +// reference to the object. template inline v8::Local PersistentToLocal( const v8::Persistent& persistent); +// If persistent.IsWeak() == false, then do not call persistent.Dispose() +// while the returned Local is still in scope, it will destroy the +// reference to the object. template inline v8::Local PersistentToLocal( v8::Isolate* isolate, diff --git a/src/node_script.cc b/src/node_script.cc index 6e3598456db..30d20022f11 100644 --- a/src/node_script.cc +++ b/src/node_script.cc @@ -174,7 +174,7 @@ Local WrappedContext::NewInstance() { Local WrappedContext::GetV8Context() { - return PersistentToLocal(context_); + return Local::New(node_isolate, context_); } @@ -405,7 +405,7 @@ void WrappedScript::EvalMachine(const FunctionCallbackInfo& args) { "'this' must be a result of previous new Script(code) call."); } - script = PersistentToLocal(n_script->script_); + script = Local