Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

async_hooks: prevent async storage methods leaking to outer context #31950

Closed
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
async_hooks: merge run and exit methods
puzpuzpuz authored and stephen.belanger committed Apr 7, 2020
commit d4a586d9c75af48092dede641009d21a0f86d894
2 changes: 1 addition & 1 deletion benchmark/async_hooks/async-resource-vs-destroy.js
Original file line number Diff line number Diff line change
@@ -101,7 +101,7 @@ function buildDestroy(getServe) {
function buildAsyncLocalStorage(getServe) {
const asyncLocalStorage = new AsyncLocalStorage();
const server = createServer((req, res) => {
asyncLocalStorage.runSyncAndReturn({}, () => {
asyncLocalStorage.run({}, () => {
getServe(getCLS, setCLS)(req, res);
});
});
146 changes: 16 additions & 130 deletions doc/api/async_hooks.md
Original file line number Diff line number Diff line change
@@ -922,7 +922,7 @@ added: v13.10.0
-->

Creates a new instance of `AsyncLocalStorage`. Store is only provided within a
`run` or a `runSyncAndReturn` method call.
`run` method call.

### `asyncLocalStorage.disable()`
<!-- YAML
@@ -931,8 +931,7 @@ added: v13.10.0

This method disables the instance of `AsyncLocalStorage`. All subsequent calls
to `asyncLocalStorage.getStore()` will return `undefined` until
`asyncLocalStorage.run()` or `asyncLocalStorage.runSyncAndReturn()`
is called again.
`asyncLocalStorage.run()` is called again.

When calling `asyncLocalStorage.disable()`, all current contexts linked to the
instance will be exited.
@@ -954,8 +953,7 @@ added: v13.10.0

This method returns the current store.
If this method is called outside of an asynchronous context initialized by
calling `asyncLocalStorage.run` or `asyncLocalStorage.runSyncAndReturn`, it will
return `undefined`.
calling `asyncLocalStorage.run`, it will return `undefined`.

### `asyncLocalStorage.enterWith(store)`
<!-- YAML
@@ -1008,90 +1006,23 @@ added: v13.10.0
* `callback` {Function}
* `...args` {any}

Calling `asyncLocalStorage.run(callback)` will create a new asynchronous
context. Within the callback function and the asynchronous operations from
the callback, `asyncLocalStorage.getStore()` will return the object or
the primitive value passed into the `store` argument (known as "the store").
This store will be persistent through the following asynchronous calls.

The callback will be ran asynchronously. Optionally, arguments can be passed
to the function. They will be passed to the callback function.

If an error is thrown by the callback function, it will not be caught by
a `try/catch` block as the callback is ran in a new asynchronous resource.
Also, the stacktrace will be impacted by the asynchronous call.

Example:

```js
const store = { id: 1 };
asyncLocalStorage.run(store, () => {
asyncLocalStorage.getStore(); // Returns the store object
someAsyncOperation(() => {
asyncLocalStorage.getStore(); // Returns the same object
});
});
asyncLocalStorage.getStore(); // Returns undefined
```

### `asyncLocalStorage.exit(callback[, ...args])`
<!-- YAML
added: v13.10.0
-->

* `callback` {Function}
* `...args` {any}

Calling `asyncLocalStorage.exit(callback)` will create a new asynchronous
context.
Within the callback function and the asynchronous operations from the callback,
`asyncLocalStorage.getStore()` will return `undefined`.

The callback will be ran asynchronously. Optionally, arguments can be passed
to the function. They will be passed to the callback function.

If an error is thrown by the callback function, it will not be caught by
a `try/catch` block as the callback is ran in a new asynchronous resource.
Also, the stacktrace will be impacted by the asynchronous call.

Example:

```js
asyncLocalStorage.run('store value', () => {
asyncLocalStorage.getStore(); // Returns 'store value'
asyncLocalStorage.exit(() => {
asyncLocalStorage.getStore(); // Returns undefined
});
asyncLocalStorage.getStore(); // Returns 'store value'
});
```

### `asyncLocalStorage.runSyncAndReturn(store, callback[, ...args])`
<!-- YAML
added: v13.10.0
-->

* `store` {any}
* `callback` {Function}
* `...args` {any}

This methods runs a function synchronously within a context and return its
return value. The store is not accessible outside of the callback function or
the asynchronous operations created within the callback.

Optionally, arguments can be passed to the function. They will be passed to
the callback function.

If the callback function throws an error, it will be thrown by
`runSyncAndReturn` too. The stacktrace will not be impacted by this call and
the context will be exited.
If the callback function throws an error, it will be thrown by `run` too.
The stacktrace will not be impacted by this call and the context will
be exited.

Example:

```js
const store = { id: 2 };
try {
asyncLocalStorage.runSyncAndReturn(store, () => {
asyncLocalStorage.run(store, () => {
asyncLocalStorage.getStore(); // Returns the store object
throw new Error();
});
@@ -1101,7 +1032,7 @@ try {
}
```

### `asyncLocalStorage.exitSyncAndReturn(callback[, ...args])`
### `asyncLocalStorage.exit(callback[, ...args])`
<!-- YAML
added: v13.10.0
-->
@@ -1116,17 +1047,17 @@ the asynchronous operations created within the callback.
Optionally, arguments can be passed to the function. They will be passed to
the callback function.

If the callback function throws an error, it will be thrown by
`exitSyncAndReturn` too. The stacktrace will not be impacted by this call and
If the callback function throws an error, it will be thrown by `exit` too.
The stacktrace will not be impacted by this call and
the context will be re-entered.

Example:

```js
// Within a call to run or runSyncAndReturn
// Within a call to run
try {
asyncLocalStorage.getStore(); // Returns the store object or value
asyncLocalStorage.exitSyncAndReturn(() => {
asyncLocalStorage.exit(() => {
asyncLocalStorage.getStore(); // Returns undefined
throw new Error();
});
@@ -1136,68 +1067,23 @@ try {
}
```

### Choosing between `run` and `runSyncAndReturn`

#### When to choose `run`

`run` is asynchronous. It is called with a callback function that
runs within a new asynchronous call. This is the most explicit behavior as
everything that is executed within the callback of `run` (including further
asynchronous operations) will have access to the store.

If an instance of `AsyncLocalStorage` is used for error management (for
instance, with `process.setUncaughtExceptionCaptureCallback`), only
exceptions thrown in the scope of the callback function will be associated
with the context.

This method is the safest as it provides strong scoping and consistent
behavior.

It cannot be promisified using `util.promisify`. If needed, the `Promise`
constructor can be used:

```js
const store = new Map(); // initialize the store
new Promise((resolve, reject) => {
asyncLocalStorage.run(store, () => {
someFunction((err, result) => {
if (err) {
return reject(err);
}
return resolve(result);
});
});
});
```

#### When to choose `runSyncAndReturn`

`runSyncAndReturn` is synchronous. The callback function will be executed
synchronously and its return value will be returned by `runSyncAndReturn`.
The store will only be accessible from within the callback
function and the asynchronous operations created within this scope.
If the callback throws an error, `runSyncAndReturn` will throw it and it will
not be associated with the context.

This method provides good scoping while being synchronous.

#### Usage with `async/await`
### Usage with `async/await`

If, within an async function, only one `await` call is to run within a context,
the following pattern should be used:

```js
async function fn() {
await asyncLocalStorage.runSyncAndReturn(new Map(), () => {
await asyncLocalStorage.run(new Map(), () => {
asyncLocalStorage.getStore().set('key', value);
return foo(); // The return value of foo will be awaited
});
}
```

In this example, the store is only available in the callback function and the
functions called by `foo`. Outside of `runSyncAndReturn`, calling `getStore`
will return `undefined`.
functions called by `foo`. Outside of `run`, calling `getStore` will return
`undefined`.

[`after` callback]: #async_hooks_after_asyncid
[`before` callback]: #async_hooks_before_asyncid
20 changes: 2 additions & 18 deletions lib/async_hooks.js
Original file line number Diff line number Diff line change
@@ -256,15 +256,15 @@ class AsyncLocalStorage {
resource[this.kResourceStore] = store;
}

runSyncAndReturn(store, callback, ...args) {
run(store, callback, ...args) {
const resource = new AsyncResource('AsyncLocalStorage');
return resource.runInAsyncScope(() => {
this.enterWith(store);
return callback(...args);
});
}

exitSyncAndReturn(callback, ...args) {
exit(callback, ...args) {
if (!this.enabled) {
return callback(...args);
}
@@ -282,22 +282,6 @@ class AsyncLocalStorage {
return resource[this.kResourceStore];
}
}

run(store, callback, ...args) {
process.nextTick(() => {
this.enterWith(store);
return callback(...args);
});
}

exit(callback, ...args) {
if (!this.enabled) {
return process.nextTick(callback, ...args);
}
this.enabled = false;
process.nextTick(callback, ...args);
this.enabled = true;
}
}

// Placing all exports down here because the exported classes won't export
9 changes: 1 addition & 8 deletions test/async-hooks/test-async-local-storage-args.js
Original file line number Diff line number Diff line change
@@ -6,15 +6,8 @@ const { AsyncLocalStorage } = require('async_hooks');
const asyncLocalStorage = new AsyncLocalStorage();

asyncLocalStorage.run({}, (runArg) => {
assert.strictEqual(runArg, 1);
asyncLocalStorage.exit((exitArg) => {
assert.strictEqual(exitArg, 2);
}, 2);
}, 1);

asyncLocalStorage.runSyncAndReturn({}, (runArg) => {
assert.strictEqual(runArg, 'foo');
asyncLocalStorage.exitSyncAndReturn((exitArg) => {
asyncLocalStorage.exit((exitArg) => {
assert.strictEqual(exitArg, 'bar');
}, 'bar');
}, 'foo');
2 changes: 1 addition & 1 deletion test/async-hooks/test-async-local-storage-async-await.js
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ async function test() {
}

async function main() {
await asyncLocalStorage.runSyncAndReturn(new Map(), test);
await asyncLocalStorage.run(new Map(), test);
assert.strictEqual(asyncLocalStorage.getStore(), undefined);
}

Original file line number Diff line number Diff line change
@@ -16,7 +16,7 @@ async function testAwait() {
await foo();
assert.notStrictEqual(asyncLocalStorage.getStore(), undefined);
assert.strictEqual(asyncLocalStorage.getStore().get('key'), 'value');
await asyncLocalStorage.exitSyncAndReturn(testOut);
await asyncLocalStorage.exit(testOut);
}

asyncLocalStorage.run(new Map(), () => {
4 changes: 2 additions & 2 deletions test/async-hooks/test-async-local-storage-enable-disable.js
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@ const { AsyncLocalStorage } = require('async_hooks');

const asyncLocalStorage = new AsyncLocalStorage();

asyncLocalStorage.runSyncAndReturn(new Map(), () => {
asyncLocalStorage.run(new Map(), () => {
asyncLocalStorage.getStore().set('foo', 'bar');
process.nextTick(() => {
assert.strictEqual(asyncLocalStorage.getStore().get('foo'), 'bar');
@@ -24,7 +24,7 @@ asyncLocalStorage.runSyncAndReturn(new Map(), () => {

process.nextTick(() => {
assert.strictEqual(asyncLocalStorage.getStore(), undefined);
asyncLocalStorage.runSyncAndReturn(new Map(), () => {
asyncLocalStorage.run(new Map(), () => {
assert.notStrictEqual(asyncLocalStorage.getStore(), undefined);
});
});
26 changes: 0 additions & 26 deletions test/async-hooks/test-async-local-storage-errors-async.js

This file was deleted.

Original file line number Diff line number Diff line change
@@ -14,7 +14,7 @@ process.setUncaughtExceptionCaptureCallback((err) => {
});

try {
asyncLocalStorage.runSyncAndReturn(new Map(), () => {
asyncLocalStorage.run(new Map(), () => {
const store = asyncLocalStorage.getStore();
store.set('hello', 'node');
setTimeout(() => {
2 changes: 1 addition & 1 deletion test/async-hooks/test-async-local-storage-gcable.js
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ const onGC = require('../common/ongc');

let asyncLocalStorage = new AsyncLocalStorage();

asyncLocalStorage.runSyncAndReturn({}, () => {
asyncLocalStorage.run({}, () => {
asyncLocalStorage.disable();

onGC(asyncLocalStorage, { ongc: common.mustCall() });
15 changes: 3 additions & 12 deletions test/async-hooks/test-async-local-storage-misc-stores.js
Original file line number Diff line number Diff line change
@@ -5,20 +5,11 @@ const { AsyncLocalStorage } = require('async_hooks');

const asyncLocalStorage = new AsyncLocalStorage();

asyncLocalStorage.run(42, () => {
assert.strictEqual(asyncLocalStorage.getStore(), 42);
asyncLocalStorage.run('hello node', () => {
assert.strictEqual(asyncLocalStorage.getStore(), 'hello node');
});

const runStore = { foo: 'bar' };
const runStore = { hello: 'node' };
asyncLocalStorage.run(runStore, () => {
assert.strictEqual(asyncLocalStorage.getStore(), runStore);
});

asyncLocalStorage.runSyncAndReturn('hello node', () => {
assert.strictEqual(asyncLocalStorage.getStore(), 'hello node');
});

const runSyncStore = { hello: 'node' };
asyncLocalStorage.runSyncAndReturn(runSyncStore, () => {
assert.strictEqual(asyncLocalStorage.getStore(), runSyncStore);
});
Loading