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: #30080

PR-URL: #30087
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
addaleax authored and targos committed Oct 28, 2019

Verified

This commit was signed with the committer’s verified signature.
Szymon20000 Szymon Kapała
1 parent 93b1bb8 commit 4458378
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
@@ -17,6 +17,7 @@ const {
executionAsyncId,
triggerAsyncId,
// Private API
hasAsyncIdStack,
getHookArrays,
enableHooks,
disableHooks,
@@ -172,7 +173,8 @@ class AsyncResource {
return fn(...args);
return Reflect.apply(fn, thisArg, args);
} finally {
emitAfter(asyncId);
if (hasAsyncIdStack())
emitAfter(asyncId);
}
}

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 4458378

Please sign in to comment.