Skip to content

Commit

Permalink
domains: clear stack when no error handler
Browse files Browse the repository at this point in the history
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: #4659
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
  • Loading branch information
Julien Gilli committed Jan 13, 2016
1 parent e98bcfa commit 90204cc
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 15 deletions.
24 changes: 9 additions & 15 deletions lib/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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,
Expand All @@ -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;
};

Expand Down
Original file line number Diff line number Diff line change
@@ -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');
});

0 comments on commit 90204cc

Please sign in to comment.