Skip to content

Commit

Permalink
process: do not directly schedule _tickCallback in _fatalException
Browse files Browse the repository at this point in the history
When a process encounters a _fatalException that is caught, it should
schedule execution of nextTicks but not in an arbitrary place of the
next Immediates queue. Instead, add a no-op function to the queue
that will ensure processImmediate runs, which will then ensure
that nextTicks are processed at the end.

Backport-PR-URL: #19006
PR-URL: #17841
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
apapirovski authored and MylesBorins committed Feb 26, 2018
1 parent d348496 commit 4acea14
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 25 deletions.
45 changes: 20 additions & 25 deletions lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,8 @@
}
}

function noop() {}

function setupProcessFatal() {
const async_wrap = process.binding('async_wrap');
// Arrays containing hook flags and ids for async_hook calls.
Expand All @@ -419,23 +421,15 @@
kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants;

process._fatalException = function(er) {
var caught;

// It's possible that kDefaultTriggerAsyncId was set for a constructor
// call that threw and was never cleared. So clear it now.
async_id_fields[kDefaultTriggerAsyncId] = -1;

if (exceptionHandlerState.captureFn !== null) {
exceptionHandlerState.captureFn(er);
caught = true;
}

if (!caught)
caught = process.emit('uncaughtException', er);

// If someone handled it, then great. otherwise, die in C++ land
// since that means that we'll exit the process, emit the 'exit' event
if (!caught) {
} else if (!process.emit('uncaughtException', er)) {
// If someone handled it, then great. otherwise, die in C++ land
// since that means that we'll exit the process, emit the 'exit' event
try {
if (!process._exiting) {
process._exiting = true;
Expand All @@ -444,24 +438,25 @@
} catch (er) {
// nothing to be done about it at this point.
}
return false;
}

// If we handled an error, then make sure any ticks get processed
// by ensuring that the next Immediate cycle isn't empty
NativeModule.require('timers').setImmediate(noop);

// Emit the after() hooks now that the exception has been handled.
if (async_hook_fields[kAfter] > 0) {
const { emitAfter } = NativeModule.require('internal/async_hooks');
do {
emitAfter(async_id_fields[kExecutionAsyncId]);
} while (async_hook_fields[kStackLength] > 0);
// Or completely empty the id stack.
} else {
// If we handled an error, then make sure any ticks get processed
NativeModule.require('timers').setImmediate(process._tickCallback);

// Emit the after() hooks now that the exception has been handled.
if (async_hook_fields[kAfter] > 0) {
do {
NativeModule.require('internal/async_hooks').emitAfter(
async_id_fields[kExecutionAsyncId]);
} while (async_hook_fields[kStackLength] > 0);
// Or completely empty the id stack.
} else {
clearAsyncIdStack();
}
clearAsyncIdStack();
}

return caught;
return true;
};
}

Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-process-fatal-exception-tick.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

const common = require('../common');
const assert = require('assert');

// If a process encounters an uncaughtException, it should schedule
// processing of nextTicks on the next Immediates cycle but not
// before all Immediates are handled

let stage = 0;

process.once('uncaughtException', common.expectsError({
type: Error,
message: 'caughtException'
}));

setImmediate(() => {
stage++;
process.nextTick(() => assert.strictEqual(stage, 2));
});
const now = Date.now();
setTimeout(() => setImmediate(() => stage++), 1);
while (now + 10 >= Date.now());
throw new Error('caughtException');

0 comments on commit 4acea14

Please sign in to comment.