domains: clear stack when no error handler

Clear domains stack __even if no domain error handler is set__ so that
code running in the process' uncaughtException handler, or any code that
may be executed when an error is thrown and not caught and that is not
the domain's error handler, doesn't run in the context of the domain
within which the error was thrown.

PR: #4659
PR-URL: https://github.com/nodejs/node/pull/4659
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

2 files changed, 31 insertions, 15 deletions

diff --git a/lib/domain.js b/lib/domain.js
index 630d02ec33..41b4859361 100644
--- a/lib/domain.js
+++ b/lib/domain.js
@@ -62,18 +62,6 @@ Domain.prototype._errorHandler = function errorHandler(er) {
   var caught = false;
   var self = this;
 
-  function emitError() {
-    var handled = self.emit('error', er);
-
-    // Exit all domains on the stack.  Uncaught exceptions end the
-    // current tick and no domains should be left on the stack
-    // between ticks.
-    stack.length = 0;
-    exports.active = process.domain = null;
-
-    return handled;
-  }
-
   // ignore errors on disposed domains.
   //
   // XXX This is a bit stupid.  We should probably get rid of
@@ -107,7 +95,7 @@ Domain.prototype._errorHandler = function errorHandler(er) {
         // 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();
+        caught = self.emit('error', er);
       } finally {
         process._emittingTopLevelDomainError = false;
       }
@@ -123,7 +111,7 @@ Domain.prototype._errorHandler = function errorHandler(er) {
       //
       // If caught is false after this, then there's no need to exit()
       // the domain, because we're going to crash the process anyway.
-      caught = emitError();
+      caught = self.emit('error', er);
     } catch (er2) {
       // The domain error handler threw!  oh no!
       // See if another domain can catch THIS error,
@@ -138,9 +126,15 @@ Domain.prototype._errorHandler = function errorHandler(er) {
       } else {
         caught = false;
       }
-      return caught;
     }
   }
+
+  // Exit all domains on the stack.  Uncaught exceptions end the
+  // current tick and no domains should be left on the stack
+  // between ticks.
+  stack.length = 0;
+  exports.active = process.domain = null;
+
   return caught;
 };
 
diff --git a/test/parallel/test-domain-stack-empty-in-process-uncaughtexception.js b/test/parallel/test-domain-stack-empty-in-process-uncaughtexception.js
new file mode 100644
index 0000000000..f4095232f4
--- /dev/null
+++ b/test/parallel/test-domain-stack-empty-in-process-uncaughtexception.js
@@ -0,0 +1,22 @@
+'use strict';
+
+const common = require('../common');
+const domain = require('domain');
+const assert = require('assert');
+
+const d = domain.create();
+
+process.on('uncaughtException', common.mustCall(function onUncaught() {
+  assert.equal(process.domain, null,
+    'domains stack should be empty in uncaughtException handler');
+}));
+
+process.on('beforeExit', common.mustCall(function onBeforeExit() {
+  assert.equal(process.domain, null,
+    'domains stack should be empty in beforeExit handler');
+}));
+
+d.run(function() {
+  throw new Error('boom');
+});
+