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>
  • Loading branch information
addaleax authored and targos committed Nov 11, 2019
1 parent ee3c3ad commit 56be32d
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 56be32d

Please sign in to comment.