domains: fix handling of uncaught exceptions

lib/domain.js

@@ -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;
 
@@ -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 (EventEmitter.listenerCount(self, 'error') > 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

src/env.h

@@ -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)                                        \

src/node.cc

@@ -908,19 +908,53 @@ 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.As<Object>();
+
+  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 TopDomainHasErrorHandler(const Environment* env) {
+  HandleScope scope(env->isolate());
+
+  if (!env->using_domains())
     return false;
 
-  Local<Value> domain_v = domain_array->Get(0);
-  return !domain_v->IsNull();
+  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();
+  if (domains_stack_length == 0)
+    return false;
+
+  Local<Value> top_domain_v =
+      domains_stack_array->Get(domains_stack_length - 1);
+
+  if (!top_domain_v->IsObject())
+    return false;
+
+  Local<Object> top_domain = top_domain_v.As<Object>();
+  if (DomainHasErrorHandler(env, top_domain))
+    return true;
+
+  return false;
 }
 
 
@@ -932,7 +966,7 @@ bool ShouldAbortOnUncaughtException(v8::Isolate* isolate) {
   bool isEmittingTopLevelDomainError =
     process_object->Get(emitting_top_level_domain_error_key)->BooleanValue();
 
-  return !IsDomainActive(env) || isEmittingTopLevelDomainError;
+  return isEmittingTopLevelDomainError || !TopDomainHasErrorHandler(env);
 }
 
 
@@ -960,8 +994,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();

test/common.js

@@ -347,3 +347,37 @@ exports.busyLoop = function busyLoop(time) {
     ;
   }
 };
+
+// Returns true if the exit code "exitCode" and/or signal name "signal"
+// represent the exit code and/or signal name of a node process that aborted,
+// false otherwise.
+exports.nodeProcessAborted = function nodeProcessAborted(exitCode, signal) {
+  // Depending on the compiler used, node will exit with either
+  // exit code 132 (SIGILL), 133 (SIGTRAP) or 134 (SIGABRT).
+  var expectedExitCodes = [132, 133, 134];
+
+  // On platforms using KSH as the default shell (like SmartOS),
+  // when a process aborts, KSH exits with an exit code that is
+  // greater than 256, and thus the exit code emitted with the 'exit'
+  // event is null and the signal is set to either SIGILL, SIGTRAP,
+  // or SIGABRT (depending on the compiler).
+  var expectedSignals = ['SIGILL', 'SIGTRAP', 'SIGABRT'];
+
+  // On Windows, v8's base::OS::Abort triggers an access violation,
+  // which corresponds to exit code 3221225477 (0xC0000005)
+  if (process.platform === 'win32')
+    expectedExitCodes = [3221225477];
+
+  // When using --abort-on-uncaught-exception, V8 will use
+  // base::OS::Abort to terminate the process.
+  // Depending on the compiler used, the shell or other aspects of
+  // the platform used to build the node binary, this will actually
+  // make V8 exit by aborting or by raising a signal. In any case,
+  // one of them (exit code or signal) needs to be set to one of
+  // the expected exit codes or signals.
+  if (signal !== null) {
+    return expectedSignals.indexOf(signal) > -1;
+  } else {
+    return expectedExitCodes.indexOf(exitCode) > -1;
+  }
+};