Skip to content

Commit

Permalink
async_hooks: only emit after for AsyncResource if stack not empty
Browse files Browse the repository at this point in the history
We clear the async id stack inside the uncaught exception handler and
emit `after` events in the process, so we should not emit `after`
a second time from the `runInAsyncScope()` code.

This should match the behaviour we have in C++.

Fixes: nodejs#30080

PR-URL: nodejs#30087
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
  • Loading branch information
addaleax committed Oct 26, 2019
1 parent cb36fa6 commit 6858c7e
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const {
executionAsyncId,
triggerAsyncId,
// Private API
hasAsyncIdStack,
getHookArrays,
enableHooks,
disableHooks,
Expand Down Expand Up @@ -172,7 +173,8 @@ class AsyncResource {
return fn(...args);
return Reflect.apply(fn, thisArg, args);
} finally {
emitAfter(asyncId);
if (hasAsyncIdStack())
emitAfter(asyncId);
}
}

Expand Down
36 changes: 36 additions & 0 deletions test/parallel/test-queue-microtask-uncaught-asynchooks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const async_hooks = require('async_hooks');

// Regression test for https://github.com/nodejs/node/issues/30080:
// An uncaught exception inside a queueMicrotask callback should not lead
// to multiple after() calls for it.

let µtaskId;
const events = [];

async_hooks.createHook({
init(id, type, triggerId, resoure) {
if (type === 'Microtask') {
µtaskId = id;
events.push('init');
}
},
before(id) {
if (id === µtaskId) events.push('before');
},
after(id) {
if (id === µtaskId) events.push('after');
},
destroy(id) {
if (id === µtaskId) events.push('destroy');
}
}).enable();

queueMicrotask(() => { throw new Error(); });

process.on('uncaughtException', common.mustCall());
process.on('exit', () => {
assert.deepStrictEqual(events, ['init', 'after', 'before', 'destroy']);
});

0 comments on commit 6858c7e

Please sign in to comment.