domains: fix handling of uncaught exceptions

Fix node exiting due to an exception being thrown rather than emitting
an 'uncaughtException' event on the process object when:
1. no error handler is set on the domain within which an error is thrown
2. an 'uncaughtException' event listener is set on the process

Also fix an issue where the process would not abort in the proper
function call if an error is thrown within a domain with no error
handler and --abort-on-uncaught-exception is used.

Fixes nodejs#3607 and nodejs#3653.

lib/domain.js

@@ -32,19 +32,20 @@ var endMethods = ['end', 'abort', 'destroy', 'destroySoon'];
 // a few side effects.
 events.usingDomains = true;
 
+// 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._usingDomains();
+process._usingDomains(stack);
 
 exports.Domain = Domain;
 
 exports.create = exports.createDomain = function(cb) {
   return new Domain(cb);
 };
 
-// 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;
 

src/node.cc

@@ -105,6 +105,8 @@ Persistent<String> domain_symbol;
 // declared in node_internals.h
 Persistent<Object> process;
 
+static Persistent<Array> domains_stack;
+
 static Persistent<Function> process_tickFromSpinner;
 static Persistent<Function> process_tickCallback;
 
@@ -126,6 +128,8 @@ static Persistent<String> exit_symbol;
 static Persistent<String> disposed_symbol;
 
 static Persistent<String> emitting_toplevel_domain_error_symbol;
+static Persistent<String> _events_symbol;
+static Persistent<String> error_symbol;
 
 static bool print_eval = false;
 static bool force_repl = false;
@@ -904,25 +908,77 @@ Handle<Value> FromConstructorTemplate(Persistent<FunctionTemplate> t,
   return scope.Close(t->GetFunction()->NewInstance(argc, argv));
 }
 
-static bool IsDomainActive() {
-  if (domain_symbol.IsEmpty())
-    domain_symbol = NODE_PSYMBOL("domain");
+static bool DomainHasErrorHandler(const Local<Object>& domain) {
+  HandleScope scope;
+
+  if (_events_symbol.IsEmpty())
+    _events_symbol = NODE_PSYMBOL("_events");
+
+  Local<Value> domain_event_listeners_v = domain->Get(_events_symbol);
+  if (!domain_event_listeners_v->IsObject())
+    return false;
+
+  Local<Object> domain_event_listeners_o =
+      domain_event_listeners_v.As<Object>();
+
+  if (error_symbol.IsEmpty())
+    error_symbol = NODE_PSYMBOL("error");
+
+  Local<Value> domain_error_listeners_v =
+      domain_event_listeners_o->Get(error_symbol);
+
+  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() {
+  HandleScope scope;
+
+  if (!using_domains)
+    return false;
 
-  Local<Value> domain_v = process->Get(domain_symbol);
+  Local<Array> domains_stack_array = Local<Array>::New(domains_stack);
+  if (domains_stack_array->Length() == 0)
+    return false;
 
-  return domain_v->IsObject();
+  uint32_t domains_stack_length = domains_stack_array->Length();
+  if (domains_stack_length == 0)
+    return false;
+
+  Local<Value> domain_v = domains_stack_array->Get(domains_stack_length - 1);
+  if (!domain_v->IsObject())
+    return false;
+
+  Local<Object> domain = domain_v.As<Object>();
+  if (DomainHasErrorHandler(domain))
+    return true;
+
+  return false;
 }
 
 bool ShouldAbortOnUncaughtException() {
   Local<Value> emitting_toplevel_domain_error_v =
     process->Get(emitting_toplevel_domain_error_symbol);
-  return !IsDomainActive() || emitting_toplevel_domain_error_v->BooleanValue();
+
+  return emitting_toplevel_domain_error_v->BooleanValue() ||
+    !TopDomainHasErrorHandler();
 }
 
 Handle<Value> UsingDomains(const Arguments& args) {
   HandleScope scope;
+
   if (using_domains)
     return scope.Close(Undefined());
+
+  if (!args[0]->IsArray()) {
+    fprintf(stderr, "domains stack must be an array\n");
+    abort();
+  }
+
   using_domains = true;
   Local<Value> tdc_v = process->Get(String::New("_tickDomainCallback"));
   Local<Value> ndt_v = process->Get(String::New("_nextDomainTick"));
@@ -934,6 +990,9 @@ Handle<Value> UsingDomains(const Arguments& args) {
     fprintf(stderr, "process._nextDomainTick assigned to non-function\n");
     abort();
   }
+
+  domains_stack = Persistent<Array>::New(args[0].As<Array>());
+
   Local<Function> tdc = tdc_v.As<Function>();
   Local<Function> ndt = ndt_v.As<Function>();
   process->Set(String::New("_tickCallback"), tdc);
@@ -2449,7 +2508,10 @@ Handle<Object> SetupProcessObject(int argc, char *argv[]) {
   process->Set(String::NewSymbol("_tickInfoBox"), info_box);
 
   // pre-set _events object for faster emit checks
-  process->Set(String::NewSymbol("_events"), Object::New());
+  if (_events_symbol.IsEmpty())
+    _events_symbol = NODE_PSYMBOL("_events");
+
+  process->Set(_events_symbol, Object::New());
 
   if (emitting_toplevel_domain_error_symbol.IsEmpty())
     emitting_toplevel_domain_error_symbol =

src/node.js

@@ -229,6 +229,7 @@
     // and exit if there are no listeners.
     process._fatalException = function(er) {
       var caught = false;
+
       if (process.domain) {
         var domain = process.domain;
         var domainModule = NativeModule.require('domain');
@@ -256,11 +257,17 @@
         // aborting in these cases.
         if (domainStack.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 = domain.emit('error', er);
+            // 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 (process.EventEmitter.listenerCount(domain, 'error') > 0) {
+              // 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 = domain.emit('error', er);
+            }
           } finally {
             process._emittingTopLevelDomainError = false;
           }
@@ -297,9 +304,12 @@
         // current tick and no domains should be left on the stack
         // between ticks.
         _clearDomainsStack();
-      } else {
+      }
+
+      if (!caught) {
         caught = process.emit('uncaughtException', er);
       }
+
       // if someone handled it, then great.  otherwise, die in C++ land
       // since that means that we'll exit the process, emit the 'exit' event
       if (!caught) {