domains: fix no error handler & abort-on-uncaught

Make the process abort if an error is thrown within a domain with no error handler and `--abort-on-uncaught-exception` is used. If the domain within which the error is thrown has no error handler, but a domain further down the domains stack has one, the process will not abort. Fixes #3653
nodejs · misterdjules · Nov 3, 2015 · Nov 4, 2015 · Nov 4, 2015 · 68ca2b1b8ee6b680da0f00d4f46d15e33cba6df9
commit 68ca2b1b8ee6b680da0f00d4f46d15e33cba6df9
diff --git a/lib/domain.js b/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;
 

diff --git a/src/env.h b/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)                                        \

diff --git a/src/node.cc b/src/node.cc
@@ -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);
@@ -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);
 }
 
 
@@ -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();

diff --git a/test/simple/test-domain-abort-on-uncaught.js b/test/simple/test-domain-abort-on-uncaught.js
@@ -0,0 +1,259 @@
+'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 firstRunOnlyTopLevelErrorHandler() {
+    var d = domain.create();
+    var d2 = domain.create();
+
+    d.on('error', function errorHandler() {
+      errorHandlerCalled = true;
+    });
+
+    d.run(function() {
+      d2.run(function() {
+        throw new Error('boom!');
+      });
+    });
+  },
+
+  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);
+    });
+
+  });
+}