1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: remove TryCatch in MakeCallback

Browse Source 
After attempting to use ReThrow() and Reset() there were cases where
firing the domain's error handlers was not happening. Or in some cases
reentering MakeCallback would still cause the domain enter callback to
abort (because the error had not been Reset yet).

In order for the script to properly stop execution when a subsequent
call to MakeCallback throws it must not be located within a TryCatch.

PR-URL: https://github.com/nodejs/node/pull/4507
Reviewed-By: Fedor Indutny <fedor@indutny.com>
This commit is contained in:
Trevor Norris 2016-02-11 13:57:26 -07:00
parent 5e30478ede
commit 95afe28fc5
3 changed files with 27 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/async-wrap.cc
View File

@ -196,33 +196,28 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
}
}
TryCatch try_catch(env()->isolate());
try_catch.SetVerbose(true);
if (has_domain) {
Local<Value> enter_v = domain->Get(env()->enter_string());
if (enter_v->IsFunction()) {
enter_v.As<Function>()->Call(domain, 0, nullptr);
if (try_catch.HasCaught())
return Undefined(env()->isolate());
if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
FatalError("node::AsyncWrap::MakeCallback",
"domain enter callback threw, please report this");
}
}
}
if (ran_init_callback() && !pre_fn.IsEmpty()) {
pre_fn->Call(context, 0, nullptr);
if (try_catch.HasCaught())
if (pre_fn->Call(context, 0, nullptr).IsEmpty())
FatalError("node::AsyncWrap::MakeCallback", "pre hook threw");
}
Local<Value> ret = cb->Call(context, argc, argv);
if (ran_init_callback() && !post_fn.IsEmpty()) {
post_fn->Call(context, 0, nullptr);
if (try_catch.HasCaught())
if (post_fn->Call(context, 0, nullptr).IsEmpty())
FatalError("node::AsyncWrap::MakeCallback", "post hook threw");
}
// If the return value is empty then the callback threw.
if (ret.IsEmpty()) {
return Undefined(env()->isolate());
}
@ -230,9 +225,10 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
if (has_domain) {
Local<Value> exit_v = domain->Get(env()->exit_string());
if (exit_v->IsFunction()) {
exit_v.As<Function>()->Call(domain, 0, nullptr);
if (try_catch.HasCaught())
return Undefined(env()->isolate());
if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
FatalError("node::AsyncWrap::MakeCallback",
"domain exit callback threw, please report this");
}
}
}
@ -251,9 +247,7 @@ Local<Value> AsyncWrap::MakeCallback(const Local<Function> cb,
return ret;
}
env()->tick_callback_function()->Call(process, 0, nullptr);
if (try_catch.HasCaught()) {
if (env()->tick_callback_function()->Call(process, 0, nullptr).IsEmpty()) {
return Undefined(env()->isolate());
}

13
src/env.cc
View File

@ -18,6 +18,7 @@ using v8::Message;
using v8::StackFrame;
using v8::StackTrace;
using v8::TryCatch;
using v8::Value;
void Environment::PrintSyncTrace() const {
if (!trace_sync_io_)
@ -80,16 +81,10 @@ bool Environment::KickNextTick(Environment::AsyncCallbackScope* scope) {
return true;
}
// process nextTicks after call
TryCatch try_catch(isolate());
try_catch.SetVerbose(true);
tick_callback_function()->Call(process_object(), 0, nullptr);
Local<Value> ret =
tick_callback_function()->Call(process_object(), 0, nullptr);
if (try_catch.HasCaught()) {
return false;
}
return true;
return !ret.IsEmpty();
}
} // namespace node

27
src/node.cc
View File

@ -1162,33 +1162,28 @@ Local<Value> MakeCallback(Environment* env,
}
}
TryCatch try_catch(env->isolate());
try_catch.SetVerbose(true);
if (has_domain) {
Local<Value> enter_v = domain->Get(env->enter_string());
if (enter_v->IsFunction()) {
enter_v.As<Function>()->Call(domain, 0, nullptr);
if (try_catch.HasCaught())
return Undefined(env->isolate());
if (enter_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
FatalError("node::MakeCallback",
"domain enter callback threw, please report this");
}
}
}
if (ran_init_callback && !pre_fn.IsEmpty()) {
pre_fn->Call(object, 0, nullptr);
if (try_catch.HasCaught())
if (pre_fn->Call(object, 0, nullptr).IsEmpty())
FatalError("node::MakeCallback", "pre hook threw");
}
Local<Value> ret = callback->Call(recv, argc, argv);
if (ran_init_callback && !post_fn.IsEmpty()) {
post_fn->Call(object, 0, nullptr);
if (try_catch.HasCaught())
if (post_fn->Call(object, 0, nullptr).IsEmpty())
FatalError("node::MakeCallback", "post hook threw");
}
// If the return value is empty then the callback threw.
if (ret.IsEmpty()) {
return Undefined(env->isolate());
}
@ -1196,14 +1191,16 @@ Local<Value> MakeCallback(Environment* env,
if (has_domain) {
Local<Value> exit_v = domain->Get(env->exit_string());
if (exit_v->IsFunction()) {
exit_v.As<Function>()->Call(domain, 0, nullptr);
if (try_catch.HasCaught())
return Undefined(env->isolate());
if (exit_v.As<Function>()->Call(domain, 0, nullptr).IsEmpty()) {
FatalError("node::MakeCallback",
"domain exit callback threw, please report this");
}
}
}
if (!env->KickNextTick(&callback_scope))
if (!env->KickNextTick(&callback_scope)) {
return Undefined(env->isolate());
}
return ret;
}