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

domains: emit uncaughtException when appropriate #3638

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
33 changes: 20 additions & 13 deletions lib/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,20 @@ Object.defineProperty(process, 'domain', {
// between js and c++ w/o much overhead
var _domain_flag = {};

// it's possible to enter one domain while already inside
// another one. the stack is each entered domain.
var stack = [];
exports._stack = stack;

// let the process know we're using domains
process._setupDomainUse(_domain, _domain_flag);
process._setupDomainUse(_domain, _domain_flag, stack);

exports.Domain = Domain;

exports.create = exports.createDomain = function() {
return new Domain();
};

// it's possible to enter one domain while already inside
// another one. the stack is each entered domain.
var stack = [];
exports._stack = stack;
// the active domain is always the one that we're currently in.
exports.active = null;

Expand Down Expand Up @@ -114,14 +115,20 @@ Domain.prototype._errorHandler = function errorHandler(er) {
// that these exceptions are caught, and thus would prevent it from
// aborting in these cases.
if (stack.length === 1) {
try {
// Set the _emittingTopLevelDomainError so that we know that, even
// if technically the top-level domain is still active, it would
// be ok to abort on an uncaught exception at this point
process._emittingTopLevelDomainError = true;
caught = emitError();
} finally {
process._emittingTopLevelDomainError = false;
// If there's no error handler, do not emit an 'error' event
// as this would throw an error, make the process exit, and thus
// prevent the process 'uncaughtException' event from being emitted
// if a listener is set.
if (self.listeners('error').length > 0) {
try {
// Set the _emittingTopLevelDomainError so that we know that, even
// if technically the top-level domain is still active, it would
// be ok to abort on an uncaught exception at this point
process._emittingTopLevelDomainError = true;
caught = emitError();
} finally {
process._emittingTopLevelDomainError = false;
}
}
} else {
// wrap this in a try/catch so we don't get infinite throwing
Expand Down
1 change: 1 addition & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ namespace node {
V(buffer_constructor_function, v8::Function) \
V(context, v8::Context) \
V(domain_array, v8::Array) \
V(domains_stack_array, v8::Array) \
V(fs_stats_constructor_function, v8::Function) \
V(gc_info_callback_function, v8::Function) \
V(module_load_list_array, v8::Array) \
Expand Down
56 changes: 47 additions & 9 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -908,21 +908,57 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
}
#endif

static bool domainHasErrorHandler(const Environment* env,
const Local<Object>& domain) {
HandleScope scope(env->isolate());

static bool IsDomainActive(const Environment* env) {
if (!env->using_domains()) {
Local<Value> domain_event_listeners_v = domain->Get(env->events_string());
if (!domain_event_listeners_v->IsObject())
return false;
}

Local<Array> domain_array = env->domain_array().As<Array>();
uint32_t domains_array_length = domain_array->Length();
if (domains_array_length == 0)
Local<Object> domain_event_listeners_o =
domain_event_listeners_v->ToObject();

if (domain_event_listeners_o->IsNull())
return false;

Local<Value> domain_v = domain_array->Get(0);
return !domain_v->IsNull();
Local<Value> domain_error_listeners_v =
domain_event_listeners_o->Get(env->error_string());

if (domain_error_listeners_v->IsFunction() ||
(domain_error_listeners_v->IsArray() &&
domain_error_listeners_v.As<Array>()->Length() > 0))
return true;

return false;
}

static bool domainsStackHasErrorHandler(const Environment* env) {
HandleScope scope(env->isolate());

if (!env->using_domains())
return false;

Local<Array> domains_stack_array = env->domains_stack_array().As<Array>();
if (domains_stack_array->Length() == 0)
return false;

uint32_t domains_stack_length = domains_stack_array->Length();
for (int i = domains_stack_length - 1; i >= 0; --i) {
Local<Value> domain_v = domains_stack_array->Get(i);
if (domain_v->IsNull())
return false;

Local<Object> domain = domain_v->ToObject();
if (domain->IsNull())
return false;

if (domainHasErrorHandler(env, domain))
return true;
}

return false;
}

bool ShouldAbortOnUncaughtException(v8::Isolate* isolate) {
Environment* env = Environment::GetCurrent(isolate);
Expand All @@ -932,7 +968,7 @@ bool ShouldAbortOnUncaughtException(v8::Isolate* isolate) {
bool isEmittingTopLevelDomainError =
process_object->Get(emitting_top_level_domain_error_key)->BooleanValue();

return !IsDomainActive(env) || isEmittingTopLevelDomainError;
return isEmittingTopLevelDomainError || !domainsStackHasErrorHandler(env);
}


Expand Down Expand Up @@ -960,8 +996,10 @@ void SetupDomainUse(const FunctionCallbackInfo<Value>& args) {

assert(args[0]->IsArray());
assert(args[1]->IsObject());
assert(args[2]->IsArray());

env->set_domain_array(args[0].As<Array>());
env->set_domains_stack_array(args[2].As<Array>());

Local<Object> domain_flag_obj = args[1].As<Object>();
Environment::DomainFlag* domain_flag = env->domain_flag();
Expand Down
Loading