async_hooks: prevent async storage methods leaking to outer context

benchmark/async_hooks/async-resource-vs-destroy.js

@@ -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);
     });
   });

doc/api/async_hooks.md

@@ -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

lib/async_hooks.js

@@ -256,18 +256,15 @@ class AsyncLocalStorage {
     resource[this.kResourceStore] = store;
   }
 
-  runSyncAndReturn(store, callback, ...args) {
-    const resource = executionAsyncResource();
-    const outerStore = resource[this.kResourceStore];
-    this.enterWith(store);
-    try {
+  run(store, callback, ...args) {
+    const resource = new AsyncResource('AsyncLocalStorage');
+    return resource.runInAsyncScope(() => {
+      this.enterWith(store);
       return callback(...args);
-    } finally {
-      resource[this.kResourceStore] = outerStore;
-    }
+    });
   }
 
-  exitSyncAndReturn(callback, ...args) {
+  exit(callback, ...args) {
     if (!this.enabled) {
       return callback(...args);
     }
@@ -285,23 +282,6 @@ class AsyncLocalStorage {
       return resource[this.kResourceStore];
     }
   }
-
-  run(store, callback, ...args) {
-    const resource = executionAsyncResource();
-    const outerStore = resource[this.kResourceStore];
-    this.enterWith(store);
-    process.nextTick(callback, ...args);
-    resource[this.kResourceStore] = outerStore;
-  }
-
-  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

test/async-hooks/test-async-local-storage-args.js

@@ -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');

test/async-hooks/test-async-local-storage-async-await.js

@@ -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);
 }
 

test/async-hooks/test-async-local-storage-async-functions.js

@@ -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(), () => {

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(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);
       });
     });

test/async-hooks/test-async-local-storage-errors-sync-ret.js → test/async-hooks/test-async-local-storage-errors.js

@@ -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(() => {

test/async-hooks/test-async-local-storage-gcable.js

@@ -10,7 +10,7 @@ const onGC = require('../common/ongc');
 
 let asyncLocalStorage = new AsyncLocalStorage();
 
-asyncLocalStorage.runSyncAndReturn({}, () => {
+asyncLocalStorage.run({}, () => {
   asyncLocalStorage.disable();
 
   onGC(asyncLocalStorage, { ongc: common.mustCall() });