From 133d061c4d27577a9eb0acb7dcf8cf4cd86bb255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= <18708370+Flarna@users.noreply.github.com> Date: Wed, 9 Nov 2022 09:27:36 +0100 Subject: [PATCH 1/8] async_hooks: add hook to stop propagation Add hook to AsyncLocalStorage to allow user to stop propagation. This is needed to avoid leaking a store if e.g. the store indicates that its operations are finished or it reached its time to live. --- doc/api/async_context.md | 21 +++++++++- lib/async_hooks.js | 25 ++++++++++-- ...st-async-local-storage-stop-propagation.js | 38 +++++++++++++++++++ 3 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 test/async-hooks/test-async-local-storage-stop-propagation.js diff --git a/doc/api/async_context.md b/doc/api/async_context.md index a41b73ce67697e..0813dd08c22daf 100644 --- a/doc/api/async_context.md +++ b/doc/api/async_context.md @@ -116,17 +116,35 @@ Each instance of `AsyncLocalStorage` maintains an independent storage context. Multiple instances can safely exist simultaneously without risk of interfering with each other's data. -### `new AsyncLocalStorage()` +### `new AsyncLocalStorage([options])` +> Stability: 1 - `options.onPropagateCb` is experimental. + +* `options` {Object} + * `onPropagateCb` {Function} Optional callback invoked before a store is + propagated to a new async resource. Returning `true` allows propagation, + returning `false` avoids it. Default is to propagate always. + Creates a new instance of `AsyncLocalStorage`. Store is only provided within a `run()` call or after an `enterWith()` call. +The `onPropagateCb` is called during creation of an async resource. Throwing at +this time will print the stack trace and exit. See +[`async_hooks` Error handling][] for details. + +Creating an async resource within the `onPropagate` callback will result in +a recursive call to `onPropagate`. + ### `asyncLocalStorage.disable()` > Stability: 1 - `options.onPropagateCb` is experimental. From 1c14ccf5f2162bd3d94ac3030a4df9029b839717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= <18708370+Flarna@users.noreply.github.com> Date: Thu, 10 Nov 2022 10:12:39 +0100 Subject: [PATCH 4/8] rename onPropagateCb to onPropagate --- doc/api/async_context.md | 8 ++++---- lib/async_hooks.js | 12 ++++++------ .../test-async-local-storage-stop-propagation.js | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/doc/api/async_context.md b/doc/api/async_context.md index 15ade1681057bb..2d76d41392a525 100644 --- a/doc/api/async_context.md +++ b/doc/api/async_context.md @@ -125,20 +125,20 @@ added: changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/45386 - description: Add option onPropagateCb. + description: Add option onPropagate. --> -> Stability: 1 - `options.onPropagateCb` is experimental. +> Stability: 1 - `options.onPropagate` is experimental. * `options` {Object} - * `onPropagateCb` {Function} Optional callback invoked before a store is + * `onPropagate` {Function} Optional callback invoked before a store is propagated to a new async resource. Returning `true` allows propagation, returning `false` avoids it. Default is to propagate always. Creates a new instance of `AsyncLocalStorage`. Store is only provided within a `run()` call or after an `enterWith()` call. -The `onPropagateCb` is called during creation of an async resource. Throwing at +The `onPropagate` is called during creation of an async resource. Throwing at this time will print the stack trace and exit. See [`async_hooks` Error handling][] for details. diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 38c8ded540f70f..bd9c95ffdf6050 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -280,16 +280,16 @@ class AsyncLocalStorage { throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); } - const { onPropagateCb = null } = options; - if (onPropagateCb !== null && typeof onPropagateCb !== 'function') { - throw new ERR_INVALID_ARG_TYPE('options.onPropagateCb', + const { onPropagate = null } = options; + if (onPropagate !== null && typeof onPropagate !== 'function') { + throw new ERR_INVALID_ARG_TYPE('options.onPropagate', 'function', - onPropagateCb); + onPropagate); } this.kResourceStore = Symbol('kResourceStore'); this.enabled = false; - this._onPropagateCb = onPropagateCb; + this._onPropagate = onPropagate; } disable() { @@ -316,7 +316,7 @@ class AsyncLocalStorage { _propagate(resource, triggerResource, type) { const store = triggerResource[this.kResourceStore]; if (this.enabled) { - if (this._onPropagateCb === null || this._onPropagateCb(type, store)) { + if (this._onPropagate === null || this._onPropagate(type, store)) { resource[this.kResourceStore] = store; } else { resource[this.kResourceStore] = undefined; diff --git a/test/async-hooks/test-async-local-storage-stop-propagation.js b/test/async-hooks/test-async-local-storage-stop-propagation.js index 6dd7890c252812..c878bfc1e9d52e 100644 --- a/test/async-hooks/test-async-local-storage-stop-propagation.js +++ b/test/async-hooks/test-async-local-storage-stop-propagation.js @@ -18,7 +18,7 @@ function onPropagate(type, store) { } const als = new AsyncLocalStorage({ - onPropagateCb: common.mustCall(onPropagate, 2) + onPropagate: common.mustCall(onPropagate, 2) }); const myStore = {}; From f49048a86c3e02e32279186bb6c2b468643eb612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= <18708370+Flarna@users.noreply.github.com> Date: Thu, 10 Nov 2022 10:17:50 +0100 Subject: [PATCH 5/8] don't set undefined on new resource --- lib/async_hooks.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index bd9c95ffdf6050..eff30dd2c8defc 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -318,8 +318,6 @@ class AsyncLocalStorage { if (this.enabled) { if (this._onPropagate === null || this._onPropagate(type, store)) { resource[this.kResourceStore] = store; - } else { - resource[this.kResourceStore] = undefined; } } } From 13b1745f5f8bab458d95b5d5a7ddcbbc22794b42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= <18708370+Flarna@users.noreply.github.com> Date: Thu, 10 Nov 2022 13:56:08 +0100 Subject: [PATCH 6/8] test wrong usage --- .../test-async-local-storage-stop-propagation.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/async-hooks/test-async-local-storage-stop-propagation.js b/test/async-hooks/test-async-local-storage-stop-propagation.js index c878bfc1e9d52e..f81acf51033536 100644 --- a/test/async-hooks/test-async-local-storage-stop-propagation.js +++ b/test/async-hooks/test-async-local-storage-stop-propagation.js @@ -36,3 +36,15 @@ als.run(myStore, common.mustCall(() => { })); })); })); + +assert.throws(() => new AsyncLocalStorage(15), { + message: 'The "options" argument must be of type object. Received type number (15)', + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' +}); + +assert.throws(() => new AsyncLocalStorage({ onPropagate: "bar" }), { + message: 'The "options.onPropagate" property must be of type function. Received type string (\'bar\')', + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError' +}); From 3c77eaf93a173653486e5999563944a8ca2db84f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= <18708370+Flarna@users.noreply.github.com> Date: Thu, 10 Nov 2022 14:52:37 +0100 Subject: [PATCH 7/8] fixup: lint --- test/async-hooks/test-async-local-storage-stop-propagation.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/async-hooks/test-async-local-storage-stop-propagation.js b/test/async-hooks/test-async-local-storage-stop-propagation.js index f81acf51033536..3b63da71cb15b6 100644 --- a/test/async-hooks/test-async-local-storage-stop-propagation.js +++ b/test/async-hooks/test-async-local-storage-stop-propagation.js @@ -43,7 +43,7 @@ assert.throws(() => new AsyncLocalStorage(15), { name: 'TypeError' }); -assert.throws(() => new AsyncLocalStorage({ onPropagate: "bar" }), { +assert.throws(() => new AsyncLocalStorage({ onPropagate: 'bar' }), { message: 'The "options.onPropagate" property must be of type function. Received type string (\'bar\')', code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError' From 358297cc045cc9637fca100838e99acf6a44977e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= Date: Mon, 14 Nov 2022 08:07:15 +0100 Subject: [PATCH 8/8] use kEmptyObject instead {} Co-authored-by: Chengzhong Wu --- lib/async_hooks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/async_hooks.js b/lib/async_hooks.js index eff30dd2c8defc..187d29281f9652 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -275,7 +275,7 @@ const storageHook = createHook({ }); class AsyncLocalStorage { - constructor(options = {}) { + constructor(options = kEmptyObject) { if (typeof options !== 'object' || options === null) { throw new ERR_INVALID_ARG_TYPE('options', 'Object', options); }