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 84e17ed56d8..c9b9a81c5b7 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 529c6cc5a83..965c64218fd 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 23f8ddde671..3797baf1832 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 91a3385e6ee..04316dff59d 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 28c8488da62..a03f803186b 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 89ac0be62c7..a0852bc1098 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 c30d72eb805..bbba8cde58d 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 c782b383e9c..b6f0b4fa742 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 f112df2b99d..3b5c57a7347 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 9f107148402..c7514d8280d 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 00000000000..56873008dd6 --- /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 38330fff607..1409a8ebc82 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 561df546d4a..3a6b352c94c 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 3b05d0f1981..0e4968534bc 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);