Skip to content

Commit

Permalink
lib: trigger uncaught exception handler for microtasks
Browse files Browse the repository at this point in the history
PR-URL: #23794
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
  • Loading branch information
devsnek authored and targos committed Oct 24, 2018
1 parent b07cb48 commit 22cd537
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 25 deletions.
4 changes: 2 additions & 2 deletions doc/api/globals.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ added: v11.0.0
* `callback` {Function} Function to be queued.

The `queueMicrotask()` method queues a microtask to invoke `callback`. If
`callback` throws an exception, the [`process` object][] `'error'` event will
be emitted.
`callback` throws an exception, the [`process` object][] `'uncaughtException'`
event will be emitted.

In general, `queueMicrotask` is the idiomatic choice over `process.nextTick()`.
`process.nextTick()` will always run before the microtask queue, and so
Expand Down
6 changes: 4 additions & 2 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
_umask, _initgroups, _setegid, _seteuid,
_setgid, _setuid, _setgroups,
_shouldAbortOnUncaughtToggle },
{ internalBinding, NativeModule }) {
{ internalBinding, NativeModule },
triggerFatalException) {
const exceptionHandlerState = { captureFn: null };
const isMainThread = internalBinding('worker').threadId === 0;

Expand Down Expand Up @@ -538,8 +539,9 @@
get: () => {
process.emitWarning('queueMicrotask() is experimental.',
'ExperimentalWarning');
const { queueMicrotask } =
const { setupQueueMicrotask } =
NativeModule.require('internal/queue_microtask');
const queueMicrotask = setupQueueMicrotask(triggerFatalException);
Object.defineProperty(global, 'queueMicrotask', {
value: queueMicrotask,
writable: true,
Expand Down
46 changes: 27 additions & 19 deletions lib/internal/queue_microtask.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,35 @@ const { AsyncResource } = require('async_hooks');
const { getDefaultTriggerAsyncId } = require('internal/async_hooks');
const { enqueueMicrotask } = internalBinding('util');

// declared separately for name, arrow function to prevent construction
const queueMicrotask = (callback) => {
if (typeof callback !== 'function') {
throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
}
const setupQueueMicrotask = (triggerFatalException) => {
const queueMicrotask = (callback) => {
if (typeof callback !== 'function') {
throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
}

const asyncResource = new AsyncResource('Microtask', {
triggerAsyncId: getDefaultTriggerAsyncId(),
requireManualDestroy: true,
});
const asyncResource = new AsyncResource('Microtask', {
triggerAsyncId: getDefaultTriggerAsyncId(),
requireManualDestroy: true,
});

enqueueMicrotask(() => {
asyncResource.runInAsyncScope(() => {
try {
callback();
} catch (e) {
process.emit('error', e);
}
enqueueMicrotask(() => {
asyncResource.runInAsyncScope(() => {
try {
callback();
} catch (error) {
// TODO(devsnek) remove this if
// https://bugs.chromium.org/p/v8/issues/detail?id=8326
// is resolved such that V8 triggers the fatal exception
// handler for microtasks
triggerFatalException(error);
} finally {
asyncResource.emitDestroy();
}
});
});
asyncResource.emitDestroy();
});
};

return queueMicrotask;
};

module.exports = { queueMicrotask };
module.exports = { setupQueueMicrotask };
19 changes: 18 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1423,6 +1423,18 @@ void FatalException(Isolate* isolate, const TryCatch& try_catch) {
}


static void FatalException(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Environment* env = Environment::GetCurrent(isolate);
if (env != nullptr && env->abort_on_uncaught_exception()) {
Abort();
}
Local<Value> exception = args[0];
Local<Message> message = Exception::CreateMessage(isolate, exception);
FatalException(isolate, exception, message);
}


static void OnMessage(Local<Message> message, Local<Value> error) {
// The current version of V8 sends messages for errors only
// (thus `error` is always set).
Expand Down Expand Up @@ -2161,14 +2173,19 @@ void LoadEnvironment(Environment* env) {
return;
}

Local<Function> trigger_fatal_exception =
env->NewFunctionTemplate(FatalException)->GetFunction(env->context())
.ToLocalChecked();

// Bootstrap Node.js
Local<Object> bootstrapper = Object::New(env->isolate());
SetupBootstrapObject(env, bootstrapper);
Local<Value> bootstrapped_node;
Local<Value> node_bootstrapper_args[] = {
env->process_object(),
bootstrapper,
bootstrapped_loaders
bootstrapped_loaders,
trigger_fatal_exception,
};
if (!ExecuteBootstrapper(env, node_bootstrapper.ToLocalChecked(),
arraysize(node_bootstrapper_args),
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-queue-microtask.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ queueMicrotask(common.mustCall(function() {
}

const eq = [];
process.on('error', (e) => {
process.on('uncaughtException', (e) => {
eq.push(e);
});

Expand Down

0 comments on commit 22cd537

Please sign in to comment.