Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: fix abort-on-uncaught-exception #3038

Closed
wants to merge 1 commit into from
Closed

src: fix abort-on-uncaught-exception #3038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 58 additions & 34 deletions lib/domain.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,39 +71,54 @@ Domain.prototype._errorHandler = function errorHandler(er) {
er.domain = this;
er.domainThrown = true;
}
// wrap this in a try/catch so we don't get infinite throwing
try {
// One of three things will happen here.
//
// 1. There is a handler, caught = true
// 2. There is no handler, caught = false
// 3. It throws, caught = false
//
// 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 = this.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;
} catch (er2) {
// The domain error handler threw! oh no!
// See if another domain can catch THIS error,
// or else crash on the original one.
// If the user already exited it, then don't double-exit.
if (this === exports.active) {
stack.pop();

if (stack.length === 1) {
try {
this.emittingTopLevelError = true;

caught = this.emit('error', er);

stack.length = 0;
exports.active = process.domain = null;
} finally {
this.emittingTopLevelError = false;
}
if (stack.length) {
exports.active = process.domain = stack[stack.length - 1];
caught = process._fatalException(er2);
} else {
caught = false;
} else {
// wrap this in a try/catch so we don't get infinite throwing
try {
// One of three things will happen here.
//
// 1. There is a handler, caught = true
// 2. There is no handler, caught = false
// 3. It throws, caught = false
//
// 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 = this.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;
} catch (er2) {
// The domain error handler threw! oh no!
// See if another domain can catch THIS error,
// or else crash on the original one.
// If the user already exited it, then don't double-exit.
if (this === exports.active) {
stack.pop();
}
if (stack.length) {
exports.active = process.domain = stack[stack.length - 1];
caught = process._fatalException(er2);
} else {
caught = false;
}
return caught;
}
return caught;
}

return caught;
};

Expand Down Expand Up @@ -176,20 +191,29 @@ Domain.prototype.run = function(fn) {
if (this._disposed)
return;

var ret;
var ret, args;

this.enter();
if (arguments.length >= 2) {
var len = arguments.length;
var args = new Array(len - 1);
args = new Array(len - 1);

for (var i = 1; i < len; i++)
args[i - 1] = arguments[i];
}

// Wrap this in a try/catch block so that
//
// domain.run(function() { throw new Error("foo"); })
//
// actually emits an error event on the domain.
try {
ret = fn.apply(this, args);
} else {
ret = fn.call(this);
} catch (err) {
process._forceTickDone();
process._fatalException(err);
}

this.exit();

return ret;
Expand Down
18 changes: 4 additions & 14 deletions lib/repl.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,20 +159,10 @@ function REPLServer(prompt,
regExMatcher.test(savedRegExMatches.join(sep));

if (!err) {
try {
if (self.useGlobal) {
result = script.runInThisContext({ displayErrors: false });
} else {
result = script.runInContext(context, { displayErrors: false });
}
} catch (e) {
err = e;
if (err && process.domain) {
debug('not recoverable, send to domain');
process.domain.emit('error', err);
process.domain.exit();
return;
}
if (self.useGlobal) {
result = script.runInThisContext({ displayErrors: false });
} else {
result = script.runInContext(context, { displayErrors: false });
}
}

Expand Down
112 changes: 80 additions & 32 deletions lib/timers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,24 +81,30 @@ function listOnTimeout() {
if (domain && domain._disposed)
continue;

try {
if (domain)
domain.enter();
threw = true;
first._called = true;
first._onTimeout();
if (domain)
domain.exit();
threw = false;
} finally {
if (threw) {
// We need to continue processing after domain error handling
// is complete, but not by using whatever domain was left over
// when the timeout threw its exception.
var oldDomain = process.domain;
process.domain = null;
process.nextTick(listOnTimeoutNT, list);
process.domain = oldDomain;
// If there's no active domain or we're in the
// error handler for the domain at the top of the stack,
// don't use try/catch to allow --abort-on-uncaught-exception
// to abort if an error is thrown.
// Otherwise, use try/catch to get the chance to emit an
// error on the current domain
if (!domain || domain.emittingTopLevelError) {
try {
fireTimer(first);
threw = false;
} finally {
if (threw) {
postFireTimer();
}
}
} else {
try {
fireTimer(first);
threw = false;
} catch (err) {
process._fatalException(err);
if (threw) {
postFireTimer();
}
}
}
}
Expand All @@ -108,6 +114,26 @@ function listOnTimeout() {
assert(L.isEmpty(list));
list.close();
delete lists[msecs];

function fireTimer(timer) {
if (domain)
domain.enter();
threw = true;
first._called = true;
first._onTimeout();
if (domain)
domain.exit();
}

function postFireTimer() {
// We need to continue processing after domain error handling
// is complete, but not by using whatever domain was left over
// when the timeout threw its exception.
var oldDomain = process.domain;
process.domain = null;
process.nextTick(listOnTimeoutNT, list);
process.domain = oldDomain;
}
}


Expand Down Expand Up @@ -370,20 +396,28 @@ function processImmediate() {
domain.enter();

var threw = true;
try {
immediate._onImmediate();
threw = false;
} finally {
if (threw) {
if (!L.isEmpty(queue)) {
// Handle any remaining on next tick, assuming we're still
// alive to do so.
while (!L.isEmpty(immediateQueue)) {
L.append(queue, L.shift(immediateQueue));
}
immediateQueue = queue;
process.nextTick(processImmediate);
}

// If there's no active domain or we're in the
// error handler for the domain at the top of the stack,
// don't use try/catch to allow --abort-on-uncaught-exception
// to abort if an error is thrown.
// Otherwise, use try/catch to get the chance to emit an
// error on the current domain
if (!domain || domain.emittingTopLevelError) {
try {
immediate._onImmediate();
threw = false;
} finally {
postImmediate(threw);
}
} else {
try {
immediate._onImmediate();
threw = false;
} catch (err) {
process._fatalException(err);

postImmediate(threw);
}
}

Expand All @@ -397,6 +431,20 @@ function processImmediate() {
if (L.isEmpty(immediateQueue)) {
process._needImmediateCallback = false;
}

function postImmediate(threw) {
if (threw) {
if (!L.isEmpty(queue)) {
// Handle any remaining on next tick, assuming we're still
// alive to do so.
while (!L.isEmpty(immediateQueue)) {
L.append(queue, L.shift(immediateQueue));
}
immediateQueue = queue;
process.nextTick(processImmediate);
}
}
}
}


Expand Down
10 changes: 5 additions & 5 deletions src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2180,11 +2180,7 @@ void FatalException(Isolate* isolate,

if (false == caught->BooleanValue()) {
ReportException(env, error, message);
if (abort_on_uncaught_exception) {
ABORT();
} else {
exit(1);
}
exit(1);
}
}

Expand Down Expand Up @@ -3201,6 +3197,10 @@ static void ParseArgs(int* argc,
} else if (strcmp(arg, "--abort-on-uncaught-exception") == 0 ||
strcmp(arg, "--abort_on_uncaught_exception") == 0) {
abort_on_uncaught_exception = true;
// Pass this option to V8 anyway, because V8 uses it to determine
// if it needs to abort on an uncaught exception
new_v8_argv[new_v8_argc] = arg;
new_v8_argc += 1;
} else if (strcmp(arg, "--v8-options") == 0) {
new_v8_argv[new_v8_argc] = "--help";
new_v8_argc += 1;
Expand Down
Loading