-
Notifications
You must be signed in to change notification settings - Fork 30k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
async_hooks: deprecate unsafe emit{Before,After}
The emit{Before,After} APIs in AsyncResource are problematic. * emit{Before,After} are named to suggest that the only thing they do is emit the before and after hooks. However, they in fact, mutate the current execution context. * They must be properly nested. Failure to do so by user code leads to catastrophic (unrecoverable) exceptions. It is very easy for the users to forget that they must be using a try/finally block around the code that must be surrounded by these operations. Even the example provided in the official docs makes this mistake. Failing to use a finally can lead to a catastrophic crash if the callback ends up throwing. This change provides a safer `runInAsyncScope` API as an alternative and deprecates emit{Before,After}. Backport-PR-URL: #18892 PR-URL: #18513 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
- Loading branch information
1 parent
5b88cb7
commit 7f5334e
Showing
6 changed files
with
160 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11 changes: 11 additions & 0 deletions
11
test/async-hooks/test-embedder.api.async-resource.runInAsyncScope.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
'use strict'; | ||
require('../common'); | ||
const assert = require('assert'); | ||
const async_hooks = require('async_hooks'); | ||
|
||
// Ensure that asyncResource.makeCallback returns the callback return value. | ||
const a = new async_hooks.AsyncResource('foobar'); | ||
const ret = a.runInAsyncScope(() => { | ||
return 1729; | ||
}); | ||
assert.strictEqual(ret, 1729); |
20 changes: 20 additions & 0 deletions
20
test/parallel/test-async-hooks-recursive-stack-runInAsyncScope.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
'use strict'; | ||
require('../common'); | ||
const assert = require('assert'); | ||
const async_hooks = require('async_hooks'); | ||
|
||
// This test verifies that the async ID stack can grow indefinitely. | ||
|
||
function recurse(n) { | ||
const a = new async_hooks.AsyncResource('foobar'); | ||
a.runInAsyncScope(() => { | ||
assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); | ||
assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); | ||
if (n >= 0) | ||
recurse(n - 1); | ||
assert.strictEqual(a.asyncId(), async_hooks.executionAsyncId()); | ||
assert.strictEqual(a.triggerAsyncId(), async_hooks.triggerAsyncId()); | ||
}); | ||
} | ||
|
||
recurse(1000); |
40 changes: 40 additions & 0 deletions
40
test/parallel/test-emit-after-uncaught-exception-runInAsyncScope.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
'use strict'; | ||
|
||
const common = require('../common'); | ||
const assert = require('assert'); | ||
const async_hooks = require('async_hooks'); | ||
|
||
const id_obj = {}; | ||
let collect = true; | ||
|
||
const hook = async_hooks.createHook({ | ||
before(id) { if (collect) id_obj[id] = true; }, | ||
after(id) { delete id_obj[id]; }, | ||
}).enable(); | ||
|
||
process.once('uncaughtException', common.mustCall((er) => { | ||
assert.strictEqual(er.message, 'bye'); | ||
collect = false; | ||
})); | ||
|
||
setImmediate(common.mustCall(() => { | ||
process.nextTick(common.mustCall(() => { | ||
assert.strictEqual(Object.keys(id_obj).length, 0); | ||
hook.disable(); | ||
})); | ||
|
||
// Create a stack of async ids that will need to be emitted in the case of | ||
// an uncaught exception. | ||
const ar1 = new async_hooks.AsyncResource('Mine'); | ||
ar1.runInAsyncScope(() => { | ||
const ar2 = new async_hooks.AsyncResource('Mine'); | ||
ar2.runInAsyncScope(() => { | ||
throw new Error('bye'); | ||
}); | ||
}); | ||
|
||
// TODO(trevnorris): This test shows that the after() hooks are always called | ||
// correctly, but it doesn't solve where the emitDestroy() is missed because | ||
// of the uncaught exception. Simple solution is to always call emitDestroy() | ||
// before the emitAfter(), but how to codify this? | ||
})); |