From 1179d417ed569dc3e8a114eeb7271d7e2a60f3b5 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Tue, 12 Jan 2016 17:48:58 -0800 Subject: [PATCH] domain: fix clearing domains stack 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, doesn't run in the context of the domain within which the error was thrown. --- lib/domain.js | 24 +++++++------------ ...tack-empty-in-process-uncaughtexception.js | 22 +++++++++++++++++ 2 files changed, 31 insertions(+), 15 deletions(-) create mode 100644 test/parallel/test-domain-stack-empty-in-process-uncaughtexception.js diff --git a/lib/domain.js b/lib/domain.js index 630d02ec332751..41b4859361d5f3 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 00000000000000..f4095232f41429 --- /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'); +}); +