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

@@ -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/simple/test-domain-abort-on-uncaught.js

@@ -0,0 +1,243 @@
+'use strict';
+
+// This test makes sure that when using --abort-on-uncaught-exception and
+// when throwing an error from within a domain that has an error handler
+// setup, the process _does not_ abort.
+
+var common = require('../common');
+var assert = require('assert');
+var domain = require('domain');
+var child_process = require('child_process');
+
+var errorHandlerCalled = false;
+
+var tests = [
+  function nextTick() {
+    var d = domain.create();
+
+    d.once('error', function(err) {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      process.nextTick(function() {
+        throw new Error('exceptional!');
+      });
+    });
+  },
+
+  function timer() {
+    var d = domain.create();
+
+    d.on('error', function(err) {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      setTimeout(function() {
+        throw new Error('exceptional!');
+      }, 33);
+    });
+  },
+
+  function immediate() {
+    console.log('starting test');
+    var d = domain.create();
+
+    d.on('error', function errorHandler() {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      setImmediate(function() {
+        throw new Error('boom!');
+      });
+    });
+  },
+
+  function timerPlusNextTick() {
+    var d = domain.create();
+
+    d.on('error', function(err) {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      setTimeout(function() {
+        process.nextTick(function() {
+          throw new Error('exceptional!');
+        });
+      }, 33);
+    });
+  },
+
+  function firstRun() {
+    var d = domain.create();
+
+    d.on('error', function(err) {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      throw new Error('exceptional!');
+    });
+  },
+
+  function fsAsync() {
+    var d = domain.create();
+
+    d.on('error', function errorHandler() {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      var fs = require('fs');
+      fs.exists('/non/existing/file', function onExists(exists) {
+        throw new Error('boom!');
+      });
+    });
+  },
+
+  function netServer() {
+    var net = require('net');
+    var d = domain.create();
+
+    d.on('error', function(err) {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      var server = net.createServer(function(conn) {
+        conn.pipe(conn);
+      });
+      server.listen(common.PORT, '0.0.0.0', function() {
+        var conn = net.connect(common.PORT, '0.0.0.0');
+        conn.once('data', function() {
+          throw new Error('ok');
+        });
+        conn.end('ok');
+        server.close();
+      });
+    });
+  },
+
+  function firstRunNestedWithErrorHandler() {
+    var d = domain.create();
+    var d2 = domain.create();
+
+    d2.on('error', function errorHandler() {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      d2.run(function() {
+        throw new Error('boom!');
+      });
+    });
+  },
+
+  function timeoutNestedWithErrorHandler() {
+    var d = domain.create();
+    var d2 = domain.create();
+
+    d2.on('error', function errorHandler() {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      d2.run(function() {
+        setTimeout(function() {
+          console.log('foo');
+          throw new Error('boom!');
+        }, 33);
+      });
+    });
+  },
+
+  function setImmediateNestedWithErrorHandler() {
+    var d = domain.create();
+    var d2 = domain.create();
+
+    d2.on('error', function errorHandler() {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      d2.run(function() {
+        setImmediate(function() {
+          throw new Error('boom!');
+        });
+      });
+    });
+  },
+
+  function nextTickNestedWithErrorHandler() {
+    var d = domain.create();
+    var d2 = domain.create();
+
+    d2.on('error', function errorHandler() {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      d2.run(function() {
+        process.nextTick(function() {
+          throw new Error('boom!');
+        });
+      });
+    });
+  },
+
+  function fsAsyncNestedWithErrorHandler() {
+    var d = domain.create();
+    var d2 = domain.create();
+
+    d2.on('error', function errorHandler() {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      d2.run(function() {
+        var fs = require('fs');
+        fs.exists('/non/existing/file', function onExists(exists) {
+          throw new Error('boom!');
+        });
+      });
+    });
+  }
+];
+
+if (process.argv[2] === 'child') {
+  var testIndex = +process.argv[3];
+
+  tests[testIndex]();
+
+  process.on('exit', function onExit() {
+    assert.equal(errorHandlerCalled, true);
+  });
+} else {
+
+  tests.forEach(function(test, testIndex) {
+    var testCmd = '';
+    if (process.platform !== 'win32') {
+      // Do not create core files, as it can take a lot of disk space on
+      // continuous testing and developers' machines
+      testCmd += 'ulimit -c 0 && ';
+    }
+
+    testCmd +=  process.argv[0];
+    testCmd += ' ' + '--abort-on-uncaught-exception';
+    testCmd += ' ' + process.argv[1];
+    testCmd += ' ' + 'child';
+    testCmd += ' ' + testIndex;
+
+    var child = child_process.exec(testCmd);
+
+    child.on('exit', function onExit(code, signal) {
+      assert.equal(code, 0, 'Test at index ' + testIndex +
+        ' should have exited with exit code 0 but instead exited with code ' +
+        code + ' and signal ' + signal);
+    });
+
+  });
+}