From 6510a741c4ed04288d56486a1384b1fbecd08eaf Mon Sep 17 00:00:00 2001 From: Andrey Pechkurov Date: Mon, 24 Feb 2020 13:00:59 +0300 Subject: [PATCH] async_hooks: add store arg in AsyncLocalStorage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit introduces store as the first argument in AsyncLocalStorage's run methods. The change is motivated by the following expectation: most users are going to use a custom object as the store and an extra Map created by the previous implementation is an overhead for their use case. Important note. This is a backwards incompatible change. It was discussed and agreed an incompatible change is ok since the API is still experimental and the modified methods were only added within the last week so usage will be minimal to none. PR-URL: https://github.com/nodejs/node/pull/31930 Reviewed-By: Stephen Belanger Reviewed-By: Vladimir de Turckheim Reviewed-By: Matteo Collina Reviewed-By: Michaël Zasso Reviewed-By: Michael Dawson --- .../async_hooks/async-resource-vs-destroy.js | 6 +-- doc/api/async_hooks.md | 46 ++++++++++--------- lib/async_hooks.js | 14 +++--- .../test-async-local-storage-args.js | 4 +- .../test-async-local-storage-async-await.js | 2 +- ...est-async-local-storage-async-functions.js | 2 +- ...test-async-local-storage-enable-disable.js | 4 +- .../test-async-local-storage-errors-async.js | 2 +- ...est-async-local-storage-errors-sync-ret.js | 2 +- .../test-async-local-storage-http.js | 2 +- .../test-async-local-storage-misc-stores.js | 24 ++++++++++ .../test-async-local-storage-nested.js | 4 +- ...est-async-local-storage-no-mix-contexts.js | 6 +-- .../test-async-local-storage-promises.js | 2 +- 14 files changed, 73 insertions(+), 47 deletions(-) create mode 100644 test/async-hooks/test-async-local-storage-misc-stores.js diff --git a/benchmark/async_hooks/async-resource-vs-destroy.js b/benchmark/async_hooks/async-resource-vs-destroy.js index 84e17ed56d8c61..c9b9a81c5b7c7f 100644 --- a/benchmark/async_hooks/async-resource-vs-destroy.js +++ b/benchmark/async_hooks/async-resource-vs-destroy.js @@ -106,7 +106,7 @@ function buildDestroy(getServe) { function buildAsyncLocalStorage(getServe) { const asyncLocalStorage = new AsyncLocalStorage(); const server = createServer((req, res) => { - asyncLocalStorage.runSyncAndReturn(() => { + asyncLocalStorage.runSyncAndReturn({}, () => { getServe(getCLS, setCLS)(req, res); }); }); @@ -118,12 +118,12 @@ function buildAsyncLocalStorage(getServe) { function getCLS() { const store = asyncLocalStorage.getStore(); - return store.get('store'); + return store.state; } function setCLS(state) { const store = asyncLocalStorage.getStore(); - store.set('store', state); + store.state = state; } function close() { diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md index 529c6cc5a83101..965c64218fdaba 100644 --- a/doc/api/async_hooks.md +++ b/doc/api/async_hooks.md @@ -893,7 +893,7 @@ function log(...args) { } http.createServer((request, response) => { - asyncLocalStorage.run(() => { + asyncLocalStorage.run(new Map(), () => { const store = asyncLocalStorage.getStore(); store.set(kReq, request); someAsyncOperation((err, result) => { @@ -943,27 +943,27 @@ in the current process. added: REPLACEME --> -* Returns: {Map} +* Returns: {any} This method returns the current store. If this method is called outside of an asynchronous context initialized by calling `asyncLocalStorage.run` or `asyncLocalStorage.runAndReturn`, it will return `undefined`. -### `asyncLocalStorage.run(callback[, ...args])` +### `asyncLocalStorage.run(store, callback[, ...args])` +* `store` {any} * `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 an instance of `Map` known as -"the store". This store will be persistent through the following -asynchronous calls. +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. @@ -975,10 +975,11 @@ Also, the stacktrace will be impacted by the asynchronous call. Example: ```js -asyncLocalStorage.run(() => { - asyncLocalStorage.getStore(); // Returns a Map +const store = { id: 1 }; +asyncLocalStorage.run(store, () => { + asyncLocalStorage.getStore(); // Returns the store object someAsyncOperation(() => { - asyncLocalStorage.getStore(); // Returns the same Map + asyncLocalStorage.getStore(); // Returns the same object }); }); asyncLocalStorage.getStore(); // Returns undefined @@ -1007,20 +1008,21 @@ Also, the stacktrace will be impacted by the asynchronous call. Example: ```js -asyncLocalStorage.run(() => { - asyncLocalStorage.getStore(); // Returns a Map +asyncLocalStorage.run('store value', () => { + asyncLocalStorage.getStore(); // Returns 'store value' asyncLocalStorage.exit(() => { asyncLocalStorage.getStore(); // Returns undefined }); - asyncLocalStorage.getStore(); // Returns the same Map + asyncLocalStorage.getStore(); // Returns 'store value' }); ``` -### `asyncLocalStorage.runSyncAndReturn(callback[, ...args])` +### `asyncLocalStorage.runSyncAndReturn(store, callback[, ...args])` +* `store` {any} * `callback` {Function} * `...args` {any} @@ -1038,9 +1040,10 @@ the context will be exited. Example: ```js +const store = { id: 2 }; try { - asyncLocalStorage.runSyncAndReturn(() => { - asyncLocalStorage.getStore(); // Returns a Map + asyncLocalStorage.runSyncAndReturn(store, () => { + asyncLocalStorage.getStore(); // Returns the store object throw new Error(); }); } catch (e) { @@ -1073,13 +1076,13 @@ Example: ```js // Within a call to run or runSyncAndReturn try { - asyncLocalStorage.getStore(); // Returns a Map + asyncLocalStorage.getStore(); // Returns the store object or value asyncLocalStorage.exitSyncAndReturn(() => { asyncLocalStorage.getStore(); // Returns undefined throw new Error(); }); } catch (e) { - asyncLocalStorage.getStore(); // Returns the same Map + asyncLocalStorage.getStore(); // Returns the same object or value // The error will be caught here } ``` @@ -1105,8 +1108,9 @@ 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(() => { + asyncLocalStorage.run(store, () => { someFunction((err, result) => { if (err) { return reject(err); @@ -1135,7 +1139,7 @@ the following pattern should be used: ```js async function fn() { - await asyncLocalStorage.runSyncAndReturn(() => { + await asyncLocalStorage.runSyncAndReturn(new Map(), () => { asyncLocalStorage.getStore().set('key', value); return foo(); // The return value of foo will be awaited }); diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 23f8ddde671e30..3797baf183250a 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -1,11 +1,9 @@ 'use strict'; const { - Map, NumberIsSafeInteger, ReflectApply, Symbol, - } = primordials; const { @@ -247,14 +245,14 @@ class AsyncLocalStorage { } } - _enter() { + _enter(store) { if (!this.enabled) { this.enabled = true; storageList.push(this); storageHook.enable(); } const resource = executionAsyncResource(); - resource[this.kResourceStore] = new Map(); + resource[this.kResourceStore] = store; } _exit() { @@ -264,8 +262,8 @@ class AsyncLocalStorage { } } - runSyncAndReturn(callback, ...args) { - this._enter(); + runSyncAndReturn(store, callback, ...args) { + this._enter(store); try { return callback(...args); } finally { @@ -289,8 +287,8 @@ class AsyncLocalStorage { } } - run(callback, ...args) { - this._enter(); + run(store, callback, ...args) { + this._enter(store); process.nextTick(callback, ...args); this._exit(); } diff --git a/test/async-hooks/test-async-local-storage-args.js b/test/async-hooks/test-async-local-storage-args.js index 91a3385e6eeb16..04316dff59d71a 100644 --- a/test/async-hooks/test-async-local-storage-args.js +++ b/test/async-hooks/test-async-local-storage-args.js @@ -5,14 +5,14 @@ const { AsyncLocalStorage } = require('async_hooks'); const asyncLocalStorage = new AsyncLocalStorage(); -asyncLocalStorage.run((runArg) => { +asyncLocalStorage.run({}, (runArg) => { assert.strictEqual(runArg, 1); asyncLocalStorage.exit((exitArg) => { assert.strictEqual(exitArg, 2); }, 2); }, 1); -asyncLocalStorage.runSyncAndReturn((runArg) => { +asyncLocalStorage.runSyncAndReturn({}, (runArg) => { assert.strictEqual(runArg, 'foo'); asyncLocalStorage.exitSyncAndReturn((exitArg) => { assert.strictEqual(exitArg, 'bar'); diff --git a/test/async-hooks/test-async-local-storage-async-await.js b/test/async-hooks/test-async-local-storage-async-await.js index 28c8488da62c53..a03f803186bdab 100644 --- a/test/async-hooks/test-async-local-storage-async-await.js +++ b/test/async-hooks/test-async-local-storage-async-await.js @@ -12,7 +12,7 @@ async function test() { } async function main() { - await asyncLocalStorage.runSyncAndReturn(test); + await asyncLocalStorage.runSyncAndReturn(new Map(), test); assert.strictEqual(asyncLocalStorage.getStore(), undefined); } diff --git a/test/async-hooks/test-async-local-storage-async-functions.js b/test/async-hooks/test-async-local-storage-async-functions.js index 89ac0be62c7488..a0852bc1098a1a 100644 --- a/test/async-hooks/test-async-local-storage-async-functions.js +++ b/test/async-hooks/test-async-local-storage-async-functions.js @@ -19,7 +19,7 @@ async function testAwait() { await asyncLocalStorage.exitSyncAndReturn(testOut); } -asyncLocalStorage.run(() => { +asyncLocalStorage.run(new Map(), () => { const store = asyncLocalStorage.getStore(); store.set('key', 'value'); testAwait(); // should not reject diff --git a/test/async-hooks/test-async-local-storage-enable-disable.js b/test/async-hooks/test-async-local-storage-enable-disable.js index c30d72eb805d5d..bbba8cde58d7e8 100644 --- a/test/async-hooks/test-async-local-storage-enable-disable.js +++ b/test/async-hooks/test-async-local-storage-enable-disable.js @@ -5,7 +5,7 @@ const { AsyncLocalStorage } = require('async_hooks'); const asyncLocalStorage = new AsyncLocalStorage(); -asyncLocalStorage.runSyncAndReturn(() => { +asyncLocalStorage.runSyncAndReturn(new Map(), () => { asyncLocalStorage.getStore().set('foo', 'bar'); process.nextTick(() => { assert.strictEqual(asyncLocalStorage.getStore().get('foo'), 'bar'); @@ -13,7 +13,7 @@ asyncLocalStorage.runSyncAndReturn(() => { assert.strictEqual(asyncLocalStorage.getStore(), undefined); process.nextTick(() => { assert.strictEqual(asyncLocalStorage.getStore(), undefined); - asyncLocalStorage.runSyncAndReturn(() => { + asyncLocalStorage.runSyncAndReturn(new Map(), () => { assert.notStrictEqual(asyncLocalStorage.getStore(), undefined); }); }); diff --git a/test/async-hooks/test-async-local-storage-errors-async.js b/test/async-hooks/test-async-local-storage-errors-async.js index c782b383e9ca95..b6f0b4fa742f61 100644 --- a/test/async-hooks/test-async-local-storage-errors-async.js +++ b/test/async-hooks/test-async-local-storage-errors-async.js @@ -13,7 +13,7 @@ process.setUncaughtExceptionCaptureCallback((err) => { assert.strictEqual(asyncLocalStorage.getStore().get('hello'), 'node'); }); -asyncLocalStorage.run(() => { +asyncLocalStorage.run(new Map(), () => { const store = asyncLocalStorage.getStore(); store.set('hello', 'node'); setTimeout(() => { diff --git a/test/async-hooks/test-async-local-storage-errors-sync-ret.js b/test/async-hooks/test-async-local-storage-errors-sync-ret.js index f112df2b99dff7..3b5c57a73472f6 100644 --- a/test/async-hooks/test-async-local-storage-errors-sync-ret.js +++ b/test/async-hooks/test-async-local-storage-errors-sync-ret.js @@ -14,7 +14,7 @@ process.setUncaughtExceptionCaptureCallback((err) => { }); try { - asyncLocalStorage.runSyncAndReturn(() => { + asyncLocalStorage.runSyncAndReturn(new Map(), () => { const store = asyncLocalStorage.getStore(); store.set('hello', 'node'); setTimeout(() => { diff --git a/test/async-hooks/test-async-local-storage-http.js b/test/async-hooks/test-async-local-storage-http.js index 9f107148402ec5..c7514d8280df35 100644 --- a/test/async-hooks/test-async-local-storage-http.js +++ b/test/async-hooks/test-async-local-storage-http.js @@ -10,7 +10,7 @@ const server = http.createServer((req, res) => { }); server.listen(0, () => { - asyncLocalStorage.run(() => { + asyncLocalStorage.run(new Map(), () => { const store = asyncLocalStorage.getStore(); store.set('hello', 'world'); http.get({ host: 'localhost', port: server.address().port }, () => { diff --git a/test/async-hooks/test-async-local-storage-misc-stores.js b/test/async-hooks/test-async-local-storage-misc-stores.js new file mode 100644 index 00000000000000..56873008dd644f --- /dev/null +++ b/test/async-hooks/test-async-local-storage-misc-stores.js @@ -0,0 +1,24 @@ +'use strict'; +require('../common'); +const assert = require('assert'); +const { AsyncLocalStorage } = require('async_hooks'); + +const asyncLocalStorage = new AsyncLocalStorage(); + +asyncLocalStorage.run(42, () => { + assert.strictEqual(asyncLocalStorage.getStore(), 42); +}); + +const runStore = { foo: 'bar' }; +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); +}); diff --git a/test/async-hooks/test-async-local-storage-nested.js b/test/async-hooks/test-async-local-storage-nested.js index 38330fff607ce2..1409a8ebc82a04 100644 --- a/test/async-hooks/test-async-local-storage-nested.js +++ b/test/async-hooks/test-async-local-storage-nested.js @@ -6,9 +6,9 @@ const { AsyncLocalStorage } = require('async_hooks'); const asyncLocalStorage = new AsyncLocalStorage(); setTimeout(() => { - asyncLocalStorage.run(() => { + asyncLocalStorage.run(new Map(), () => { const asyncLocalStorage2 = new AsyncLocalStorage(); - asyncLocalStorage2.run(() => { + asyncLocalStorage2.run(new Map(), () => { const store = asyncLocalStorage.getStore(); const store2 = asyncLocalStorage2.getStore(); store.set('hello', 'world'); diff --git a/test/async-hooks/test-async-local-storage-no-mix-contexts.js b/test/async-hooks/test-async-local-storage-no-mix-contexts.js index 561df546d4aa45..3a6b352c94ceee 100644 --- a/test/async-hooks/test-async-local-storage-no-mix-contexts.js +++ b/test/async-hooks/test-async-local-storage-no-mix-contexts.js @@ -7,8 +7,8 @@ const asyncLocalStorage = new AsyncLocalStorage(); const asyncLocalStorage2 = new AsyncLocalStorage(); setTimeout(() => { - asyncLocalStorage.run(() => { - asyncLocalStorage2.run(() => { + asyncLocalStorage.run(new Map(), () => { + asyncLocalStorage2.run(new Map(), () => { const store = asyncLocalStorage.getStore(); const store2 = asyncLocalStorage2.getStore(); store.set('hello', 'world'); @@ -28,7 +28,7 @@ setTimeout(() => { }, 100); setTimeout(() => { - asyncLocalStorage.run(() => { + asyncLocalStorage.run(new Map(), () => { const store = asyncLocalStorage.getStore(); store.set('hello', 'earth'); setTimeout(() => { diff --git a/test/async-hooks/test-async-local-storage-promises.js b/test/async-hooks/test-async-local-storage-promises.js index 3b05d0f1981a3c..0e4968534bc3e2 100644 --- a/test/async-hooks/test-async-local-storage-promises.js +++ b/test/async-hooks/test-async-local-storage-promises.js @@ -12,7 +12,7 @@ async function main() { throw err; }); await new Promise((resolve, reject) => { - asyncLocalStorage.run(() => { + asyncLocalStorage.run(new Map(), () => { const store = asyncLocalStorage.getStore(); store.set('a', 1); next().then(resolve, reject);