async_hooks: discourage AsyncLocalStorage.disable

When `--async-context-frame` is enabled (and by default in v24), there
should be no need to manually call `asyncLocalStorage.disable()`.

Author: legendecas

doc/api/async_context.md

@@ -224,13 +224,14 @@ to `asyncLocalStorage.getStore()` will return `undefined` until
 When calling `asyncLocalStorage.disable()`, all current contexts linked to the
 instance will be exited.
 
-Calling `asyncLocalStorage.disable()` is required before the
-`asyncLocalStorage` can be garbage collected. This does not apply to stores
-provided by the `asyncLocalStorage`, as those objects are garbage collected
-along with the corresponding async resources.
+There is no need to call this method in order to get an `asyncLocalStorage`
+instance from being garbage-collected.
 
-Use this method when the `asyncLocalStorage` is not in use anymore
-in the current process.
+When running Node.js with CLI flag [`--no-async-context-frame`][], calling
+`asyncLocalStorage.disable()` is required before the `asyncLocalStorage` itself
+can be garbage collected. However, this does not apply to stores provided by
+the `asyncLocalStorage`, as those objects are garbage collected along with the
+corresponding async resources.
 
 ### `asyncLocalStorage.getStore()`
 
@@ -905,6 +906,7 @@ const server = createServer((req, res) => {
 }).listen(3000);
 ```
 
+[`--no-async-context-frame`]: cli.md#--no-async-context-frame
 [`AsyncResource`]: #class-asyncresource
 [`EventEmitter`]: events.md#class-eventemitter
 [`Stream`]: stream.md#stream

lib/internal/async_local_storage/async_context_frame.js

@@ -2,6 +2,7 @@
 
 const {
   ReflectApply,
+  Symbol,
 } = primordials;
 
 const {
@@ -12,6 +13,7 @@ const AsyncContextFrame = require('internal/async_context_frame');
 const { AsyncResource } = require('async_hooks');
 
 class AsyncLocalStorage {
+  #frameKey;
   #defaultValue = undefined;
   #name = undefined;
 
@@ -30,6 +32,8 @@ class AsyncLocalStorage {
     if (options.name !== undefined) {
       this.#name = `${options.name}`;
     }
+
+    this.#frameKey = Symbol(this.#name ? `AsyncLocalStorage ${this.#name}` : 'AsyncLocalStorage');
   }
 
   /** @type {string} */
@@ -44,11 +48,13 @@ class AsyncLocalStorage {
   }
 
   disable() {
-    AsyncContextFrame.disable(this);
+    // TODO(legendecas): deprecate this method once `--async-context-frame` is
+    // the only way to use AsyncLocalStorage.
+    AsyncContextFrame.disable(this.#frameKey);
   }
 
   enterWith(data) {
-    const frame = new AsyncContextFrame(this, data);
+    const frame = new AsyncContextFrame(this.#frameKey, data);
     AsyncContextFrame.set(frame);
   }
 
@@ -68,10 +74,10 @@ class AsyncLocalStorage {
 
   getStore() {
     const frame = AsyncContextFrame.current();
-    if (!frame?.has(this)) {
+    if (!frame?.has(this.#frameKey)) {
       return this.#defaultValue;
     }
-    return frame?.get(this);
+    return frame?.get(this.#frameKey);
   }
 }
 

test/parallel/test-async-local-storage-gc.js

@@ -0,0 +1,34 @@
+// Flags: --async-context-frame --expose-gc --expose-internals
+
+'use strict';
+const common = require('../common');
+const { gcUntil } = require('../common/gc');
+const { AsyncLocalStorage } = require('async_hooks');
+const AsyncContextFrame = require('internal/async_context_frame');
+
+// To be compatible with `test-without-async-context-frame.mjs`.
+if (!AsyncContextFrame.enabled) {
+  common.skip('AsyncContextFrame is not enabled');
+}
+
+// Verify that the AsyncLocalStorage object can be garbage collected even without
+// `asyncLocalStorage.disable()` being called, when `--async-context-frame` is enabled.
+
+let weakRef = null;
+{
+  const alsValue = {};
+  let als = new AsyncLocalStorage();
+  als.run(alsValue, () => {
+    setInterval(() => {
+      /**
+       * Empty interval to keep the als value alive.
+       */
+    }, 1000).unref();
+  });
+  weakRef = new WeakRef(als);
+  als = null;
+}
+
+gcUntil('AsyncLocalStorage object can be garbage collected', () => {
+  return weakRef.deref() === undefined;
+});