async_hooks: remove legacy AsyncLocalStorage implementation

Remove the async_hooks-based AsyncLocalStorage fallback and the
--no-async-context-frame flag, making AsyncContextFrame the sole
implementation.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/63641
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>

doc/api/cli.md

@@ -2277,17 +2277,6 @@ Disable the `node-addons` exports condition as well as disable loading
 native addons. When `--no-addons` is specified, calling `process.dlopen` or
 requiring a native C++ addon will fail and throw an exception.
 
-### `--no-async-context-frame`
-
-<!-- YAML
-added: v24.0.0
--->
-
-Disables the use of [`AsyncLocalStorage`][] backed by `AsyncContextFrame` and
-uses the prior implementation which relied on async\_hooks. The previous model
-is retained for compatibility with Electron and for cases where the context
-flow may differ. However, if a difference in flow is found please report it.
-
 ### `--no-deprecation`
 
 <!-- YAML
@@ -4317,7 +4306,6 @@ one is included in the list below.
 * `--max-old-space-size-percentage`
 * `--network-family-autoselection-attempt-timeout`
 * `--no-addons`
-* `--no-async-context-frame`
 * `--no-deprecation`
 * `--no-experimental-ffi`
 * `--no-experimental-global-navigator`
@@ -4937,7 +4925,6 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
 [`--use-env-proxy`]: #--use-env-proxy
 [`--use-system-ca`]: #--use-system-ca
 [`--watch`]: #--watch
-[`AsyncLocalStorage`]: async_context.md#class-asynclocalstorage
 [`Buffer`]: buffer.md#class-buffer
 [`CRYPTO_secure_malloc_init`]: https://www.openssl.org/docs/man3.0/man3/CRYPTO_secure_malloc_init.html
 [`ERR_INVALID_TYPESCRIPT_SYNTAX`]: errors.md#err_invalid_typescript_syntax

doc/node-config-schema.json

@@ -172,10 +172,6 @@
               "type": "boolean",
               "description": "allow worker threads when any permissions are set"
             },
-            "async-context-frame": {
-              "type": "boolean",
-              "description": "Improve AsyncLocalStorage performance with AsyncContextFrame"
-            },
             "bench-isolation": {
               "type": "string",
               "description": "configures the type of benchmark isolation used in the benchmark runner"

doc/node.1

@@ -1212,12 +1212,6 @@ Disable the \fBnode-addons\fR exports condition as well as disable loading
 native addons. When \fB--no-addons\fR is specified, calling \fBprocess.dlopen\fR or
 requiring a native C++ addon will fail and throw an exception.
 .
-.It Fl -no-async-context-frame
-Disables the use of \fBAsyncLocalStorage\fR backed by \fBAsyncContextFrame\fR and
-uses the prior implementation which relied on async_hooks. The previous model
-is retained for compatibility with Electron and for cases where the context
-flow may differ. However, if a difference in flow is found please report it.
-.
 .It Fl -no-deprecation
 Silence deprecation warnings.
 .
@@ -2335,8 +2329,6 @@ one is included in the list below.
 .It
 \fB--no-addons\fR
 .It
-\fB--no-async-context-frame\fR
-.It
 \fB--no-deprecation\fR
 .It
 \fB--no-experimental-ffi\fR

lib/async_hooks.js

@@ -287,9 +287,7 @@ class AsyncResource {
 module.exports = {
   // Public API
   get AsyncLocalStorage() {
-    return AsyncContextFrame.enabled ?
-      require('internal/async_local_storage/async_context_frame') :
-      require('internal/async_local_storage/async_hooks');
+    return require('internal/async_local_storage/async_context_frame');
   },
   createHook,
   executionAsyncId,

lib/internal/async_context_frame.js

@@ -9,63 +9,34 @@ const {
   setContinuationPreservedEmbedderData,
 } = internalBinding('async_context_frame');
 
-function activeCurrent() {
-  return getContinuationPreservedEmbedderData();
-}
-
-function activeSet(frame) {
-  setContinuationPreservedEmbedderData(frame);
-}
-
-function activeExchange(frame) {
-  const prior = getContinuationPreservedEmbedderData();
-  setContinuationPreservedEmbedderData(frame);
-  return prior;
-}
-
-function activeDisable(store) {
-  getContinuationPreservedEmbedderData()?.disable(store);
-}
-
-class InactiveAsyncContextFrame extends SafeMap {
-  static get enabled() {
-    enabled_ ??= checkEnabled();
-    return enabled_;
-  }
-
-  static current() {}
-  static set(frame) {}
-  static exchange(frame) {}
-  static disable(store) {}
-}
-
-class AsyncContextFrame extends InactiveAsyncContextFrame {
+class AsyncContextFrame extends SafeMap {
   constructor(store, data) {
     super(AsyncContextFrame.current());
     this.set(store, data);
   }
 
+  static current() {
+    return getContinuationPreservedEmbedderData();
+  }
+
+  static set(frame) {
+    setContinuationPreservedEmbedderData(frame);
+  }
+
+  static exchange(frame) {
+    const prior = this.current();
+    this.set(frame);
+    return prior;
+  }
+
+  static disable(store) {
+    const frame = this.current();
+    frame?.disable(store);
+  }
+
   disable(store) {
     this.delete(store);
   }
 }
 
-let enabled_;
-
-function checkEnabled() {
-  const enabled = require('internal/options')
-    .getOptionValue('--async-context-frame');
-
-  // If enabled, install the active implementations directly. We use props
-  // rather than a prototype replacement to preserve V8 optimizations.
-  if (enabled) {
-    AsyncContextFrame.current = activeCurrent;
-    AsyncContextFrame.set = activeSet;
-    AsyncContextFrame.exchange = activeExchange;
-    AsyncContextFrame.disable = activeDisable;
-  }
-
-  return enabled;
-}
-
 module.exports = AsyncContextFrame;

lib/internal/async_hooks.js

@@ -105,7 +105,6 @@ const before_symbol = Symbol('before');
 const after_symbol = Symbol('after');
 const destroy_symbol = Symbol('destroy');
 const promise_resolve_symbol = Symbol('promiseResolve');
-const async_local_storage_context_symbol = Symbol('kAsyncLocalStorageContext');
 const emitBeforeNative = emitHookFactory(before_symbol, 'emitBeforeNative');
 const emitAfterNative = emitHookFactory(after_symbol, 'emitAfterNative');
 const emitDestroyNative = emitHookFactory(destroy_symbol, 'emitDestroyNative');
@@ -585,7 +584,7 @@ module.exports = {
   symbols: {
     async_id_symbol, trigger_async_id_symbol,
     init_symbol, before_symbol, after_symbol, destroy_symbol,
-    promise_resolve_symbol, async_local_storage_context_symbol,
+    promise_resolve_symbol,
     owner_symbol,
   },
   constants: {

lib/internal/async_local_storage/async_hooks.js

@@ -1,168 +0,0 @@
-'use strict';
-
-const {
-  ArrayPrototypeIndexOf,
-  ArrayPrototypePush,
-  ArrayPrototypeSplice,
-  ObjectIs,
-  ReflectApply,
-  Symbol,
-} = primordials;
-
-const {
-  validateObject,
-} = require('internal/validators');
-const {
-  symbols: {
-    async_local_storage_context_symbol,
-  },
-} = require('internal/async_hooks');
-
-const {
-  AsyncResource,
-  createHook,
-  executionAsyncResource,
-} = require('async_hooks');
-
-const RunScope = require('internal/async_local_storage/run_scope');
-const { kEmptyObject } = require('internal/util');
-
-const storageList = [];
-
-function getOrCreateResourceStore(resource) {
-  return resource[async_local_storage_context_symbol] ??= { __proto__: null };
-}
-
-const storageHook = createHook({
-  init(asyncId, type, triggerAsyncId, resource) {
-    const currentResource = executionAsyncResource();
-    // Value of currentResource is always a non null object
-    for (let i = 0; i < storageList.length; ++i) {
-      storageList[i]._propagate(resource, currentResource, type);
-    }
-  },
-});
-
-class AsyncLocalStorage {
-  #defaultValue = undefined;
-  #name = undefined;
-
-  /**
-   * @typedef {object} AsyncLocalStorageOptions
-   * @property {any} [defaultValue] - The default value to use when no value is set.
-   * @property {string} [name] - The name of the storage.
-   */
-  /**
-   * @param {AsyncLocalStorageOptions} [options]
-   */
-  constructor(options = kEmptyObject) {
-    this.kResourceStore = Symbol('kResourceStore');
-    this.enabled = false;
-    validateObject(options, 'options');
-    this.#defaultValue = options.defaultValue;
-
-    if (options.name !== undefined) {
-      this.#name = `${options.name}`;
-    }
-
-    this._enable();
-  }
-
-  /** @type {string} */
-  get name() { return this.#name || ''; }
-
-  static bind(fn) {
-    return AsyncResource.bind(fn);
-  }
-
-  static snapshot() {
-    return AsyncLocalStorage.bind((cb, ...args) => cb(...args));
-  }
-
-  disable() {
-    if (this.enabled) {
-      this.enabled = false;
-      // If this.enabled, the instance must be in storageList
-      const index = ArrayPrototypeIndexOf(storageList, this);
-      ArrayPrototypeSplice(storageList, index, 1);
-      if (storageList.length === 0) {
-        storageHook.disable();
-      }
-    }
-  }
-
-  _enable() {
-    if (!this.enabled) {
-      this.enabled = true;
-      ArrayPrototypePush(storageList, this);
-      storageHook.enable();
-    }
-  }
-
-  // Propagate the context from a parent resource to a child one
-  _propagate(resource, triggerResource, type) {
-    const store = triggerResource[async_local_storage_context_symbol]?.[this.kResourceStore];
-    if (this.enabled) {
-      const resourceStore = getOrCreateResourceStore(resource);
-      resourceStore[this.kResourceStore] = store;
-    }
-  }
-
-  enterWith(store) {
-    this._enable();
-    const resource = executionAsyncResource();
-    const resourceStore = getOrCreateResourceStore(resource);
-    resourceStore[this.kResourceStore] = store;
-  }
-
-  run(store, callback, ...args) {
-    // Avoid creation of an AsyncResource if store is already active
-    if (ObjectIs(store, this.getStore())) {
-      return ReflectApply(callback, null, args);
-    }
-
-    this._enable();
-
-    const resource = executionAsyncResource();
-    const resourceStore = getOrCreateResourceStore(resource);
-    const oldStore = resourceStore[this.kResourceStore];
-
-    resourceStore[this.kResourceStore] = store;
-
-    try {
-      return ReflectApply(callback, null, args);
-    } finally {
-      resourceStore[this.kResourceStore] = oldStore;
-    }
-  }
-
-  exit(callback, ...args) {
-    if (!this.enabled) {
-      return ReflectApply(callback, null, args);
-    }
-    this.disable();
-    try {
-      return ReflectApply(callback, null, args);
-    } finally {
-      this._enable();
-    }
-  }
-
-  getStore() {
-    if (this.enabled) {
-      const resource = executionAsyncResource();
-      const resourceStore = resource[async_local_storage_context_symbol];
-      if (resourceStore === undefined || !(this.kResourceStore in resourceStore)) {
-        return this.#defaultValue;
-      }
-      return resourceStore[this.kResourceStore];
-    }
-    return this.#defaultValue;
-  }
-
-  withScope(store) {
-    return new RunScope(this, store);
-  }
-}
-
-module.exports = AsyncLocalStorage;

lib/internal/timers.js

@@ -100,9 +100,6 @@ const {
   emitBefore,
   emitAfter,
   emitDestroy,
-  symbols: {
-    async_local_storage_context_symbol,
-  },
 } = require('internal/async_hooks');
 
 // Symbols for storing async id state.
@@ -132,12 +129,8 @@ const AsyncContextFrame = require('internal/async_context_frame');
 const async_context_frame = Symbol('kAsyncContextFrame');
 
 function removeStoresFromResource(resource) {
-  if (AsyncContextFrame.enabled) {
-    if (resource[async_context_frame] !== undefined) {
-      resource[async_context_frame] = undefined;
-    }
-  } else if (resource[async_local_storage_context_symbol] !== undefined) {
-    resource[async_local_storage_context_symbol] = undefined;
+  if (resource[async_context_frame] !== undefined) {
+    resource[async_context_frame] = undefined;
   }
 }
 

src/async_context_frame.cc

@@ -38,10 +38,6 @@ Local<Value> current(Isolate* isolate) {
 }
 
 void set(Environment* env, Local<Value> value) {
-  if (!env->async_context_frame_enabled()) {
-    return;
-  }
-
   env->isolate()->SetContinuationPreservedEmbedderDataV2(value);
 }
 

src/env-inl.h

@@ -460,10 +460,6 @@ inline std::shared_ptr<EnvironmentOptions> Environment::options() {
   return options_;
 }
 
-inline bool Environment::async_context_frame_enabled() const {
-  return options_->async_context_frame;
-}
-
 inline const std::vector<std::string>& Environment::argv() {
   return argv_;
 }

src/env.h

@@ -1112,7 +1112,6 @@ class Environment final : public MemoryRetainer {
                                  void* data);
 
   inline std::shared_ptr<EnvironmentOptions> options();
-  inline bool async_context_frame_enabled() const;
   inline std::shared_ptr<ExclusiveAccess<HostPort>> inspector_host_port();
 
   inline int64_t stack_trace_limit() const;

src/node_options.cc

@@ -950,11 +950,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
   AddOption(
       "--experimental-wasi-unstable-preview1", "", NoOp{}, kAllowedInEnvvar);
   AddOption("--expose-gc", "expose gc extension", V8Option{}, kAllowedInEnvvar);
-  AddOption("--async-context-frame",
-            "Improve AsyncLocalStorage performance with AsyncContextFrame",
-            BOOL_FIELD(async_context_frame),
-            kAllowedInEnvvar,
-            true);
   AddOption("--expose-internals", "", BOOL_FIELD(expose_internals));
   AddOption("--frozen-intrinsics",
             "experimental frozen intrinsics support",

src/node_options.h

@@ -238,7 +238,6 @@ class EnvironmentOptions : public Options {
   DEFINE_BOOL_FIELD(allow_openssl_store) = false;
   DEFINE_BOOL_FIELD(allow_worker_threads) = false;
   DEFINE_BOOL_FIELD(experimental_vm_modules) = EXPERIMENTALS_DEFAULT_VALUE;
-  DEFINE_BOOL_FIELD(async_context_frame) = true;
   DEFINE_BOOL_FIELD(expose_internals) = false;
   DEFINE_BOOL_FIELD(force_node_api_uncaught_exceptions_policy) = false;
   DEFINE_BOOL_FIELD(frozen_intrinsics) = false;

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

@@ -1,12 +1,11 @@
 'use strict';
-// Flags: --expose_gc --expose-internals
+// Flags: --expose_gc
 
 // This test ensures that AsyncLocalStorage gets gced once it was disabled
 // and no strong references remain in userland.
 
 const common = require('../common');
 const { AsyncLocalStorage } = require('async_hooks');
-const AsyncContextFrame = require('internal/async_context_frame');
 const { onGC } = require('../common/gc');
 
 let asyncLocalStorage = new AsyncLocalStorage();
@@ -17,11 +16,9 @@ asyncLocalStorage.run({}, common.mustCall(() => {
   onGC(asyncLocalStorage, { ongc: common.mustCall() });
 }));
 
-if (AsyncContextFrame.enabled) {
-  // This disable() is needed to remove reference form AsyncContextFrame
-  // created during exit of run() to the AsyncLocalStore instance.
-  asyncLocalStorage.disable();
-}
+// This disable() is needed to remove reference from AsyncContextFrame
+// created during exit of run() to the AsyncLocalStore instance.
+asyncLocalStorage.disable();
 
 asyncLocalStorage = null;
 global.gc();

test/parallel/test-als-defaultvalue-original.js

@@ -1,37 +0,0 @@
-// Flags: --no-async-context-frame
-'use strict';
-
-require('../common');
-
-const {
-  AsyncLocalStorage,
-} = require('async_hooks');
-
-const assert = require('assert');
-
-// ============================================================================
-// The defaultValue option
-const als1 = new AsyncLocalStorage();
-assert.strictEqual(als1.getStore(), undefined);
-
-const als2 = new AsyncLocalStorage({ defaultValue: 'default' });
-assert.strictEqual(als2.getStore(), 'default');
-
-const als3 = new AsyncLocalStorage({ defaultValue: 42 });
-assert.strictEqual(als3.getStore(), 42);
-
-const als4 = new AsyncLocalStorage({ defaultValue: null });
-assert.strictEqual(als4.getStore(), null);
-
-assert.throws(() => new AsyncLocalStorage(null), {
-  code: 'ERR_INVALID_ARG_TYPE',
-});
-
-// ============================================================================
-// The name option
-
-const als5 = new AsyncLocalStorage({ name: 'test' });
-assert.strictEqual(als5.name, 'test');
-
-const als6 = new AsyncLocalStorage();
-assert.strictEqual(als6.name, '');

test/parallel/test-als-defaultvalue.js

@@ -1,4 +1,3 @@
-// Flags: --async-context-frame
 'use strict';
 
 require('../common');

test/parallel/test-internal-async-context-frame-disable.js

@@ -1,9 +0,0 @@
-// Flags: --expose-internals --no-async-context-frame
-'use strict';
-
-require('../common');
-const assert = require('assert');
-const AsyncContextFrame = require('internal/async_context_frame');
-
-// Test that AsyncContextFrame can be disabled.
-assert(!AsyncContextFrame.enabled);

test/parallel/test-internal-async-context-frame-enabled.js

@@ -1,9 +0,0 @@
-// Flags: --expose-internals
-'use strict';
-
-require('../common');
-const assert = require('assert');
-const AsyncContextFrame = require('internal/async_context_frame');
-
-// Test that AsyncContextFrame is enabled by default.
-assert(AsyncContextFrame.enabled);

test/parallel/test-stream-finished-async-local-storage.js

@@ -1,12 +1,9 @@
-// Flags: --expose-internals
 'use strict';
 
 const common = require('../common');
 const { Readable, finished } = require('stream');
 const { AsyncLocalStorage } = require('async_hooks');
 const assert = require('assert');
-const AsyncContextFrame = require('internal/async_context_frame');
-const { enabledHooksExist } = require('internal/async_hooks');
 
 // This test verifies that ALS context is preserved when using stream.finished()
 
@@ -15,7 +12,6 @@ const readable = new Readable();
 
 als.run('test-context-1', common.mustCall(() => {
   finished(readable, common.mustCall(() => {
-    assert.strictEqual(AsyncContextFrame.enabled || enabledHooksExist(), true);
     assert.strictEqual(als.getStore(), 'test-context-1');
   }));
 }));

test/parallel/test-stream-finished-default-path.js

@@ -1,22 +0,0 @@
-// Flags: --expose-internals --no-async-context-frame
-'use strict';
-
-const common = require('../common');
-const { Readable, finished } = require('stream');
-const assert = require('assert');
-const AsyncContextFrame = require('internal/async_context_frame');
-const { enabledHooksExist } = require('internal/async_hooks');
-
-// This test verifies that when there are no active async hooks, stream.finished() uses the default callback path
-
-const readable = new Readable();
-
-finished(readable, common.mustCall(() => {
-  assert.strictEqual(enabledHooksExist(), false);
-  assert.strictEqual(
-    AsyncContextFrame.current() || enabledHooksExist(),
-    false,
-  );
-}));
-
-readable.destroy();

test/parallel/test-timers-async-store-leak.js

@@ -2,61 +2,20 @@
 'use strict';
 
 const common = require('../common');
-const { spawnSyncAndAssert } = require('../common/child_process');
 const { AsyncLocalStorage } = require('async_hooks');
 const assert = require('assert');
-
-if (process.argv[2] !== 'child') {
-  // Test with async-context-frame disabled (legacy ALS mode)
-  spawnSyncAndAssert(process.execPath, [
-    '--expose-internals',
-    '--expose-gc',
-    '--no-async-context-frame',
-    __filename,
-    'child',
-  ], {});
-
-  // Test with async-context-frame enabled (default)
-  spawnSyncAndAssert(process.execPath, [
-    '--expose-internals',
-    '--expose-gc',
-    __filename,
-    'child',
-  ], {});
-}
-
-const AsyncContextFrame = require('internal/async_context_frame');
 const {
   async_context_frame,
 } = require('internal/timers');
-const {
-  symbols: {
-    async_local_storage_context_symbol,
-  },
-} = require('internal/async_hooks');
-
-// When async-context-frame is enabled, stores are stored in the async context
-// frame, not directly on the resource. The resource holds a reference to the
-// frame via async_context_frame.
-const isACFEnabled = AsyncContextFrame.enabled;
-
-function getStore(resource, als) {
-  if (isACFEnabled) {
-    return resource[async_context_frame]?.get(als);
-  }
-  return resource[async_local_storage_context_symbol]?.[als.kResourceStore];
-}
 
+// Stores live in the async context frame, not directly on the resource. The
+// resource holds a reference to the frame via async_context_frame.
 function assertStore(resource, als, store) {
-  assert.strictEqual(getStore(resource, als), store);
+  assert.strictEqual(resource[async_context_frame]?.get(als), store);
 }
 
 function assertNoStore(resource) {
-  if (isACFEnabled) {
-    assert.strictEqual(resource[async_context_frame], undefined);
-  } else {
-    assert.strictEqual(resource[async_local_storage_context_symbol], undefined);
-  }
+  assert.strictEqual(resource[async_context_frame], undefined);
 }
 
 // Test that setTimeout does not retain a reference to the async store after

test/sequential/test-without-async-context-frame.mjs

@@ -1,24 +0,0 @@
-import { isWindows } from '../common/index.mjs';
-import { spawn } from 'node:child_process';
-import { once } from 'node:events';
-import { fileURLToPath } from 'node:url';
-import assert from 'node:assert';
-
-const python = process.env.PYTHON || (isWindows ? 'python' : 'python3');
-
-const testRunner = fileURLToPath(
-  new URL('../../tools/test.py', import.meta.url)
-);
-
-const proc = spawn(python, [
-  testRunner,
-  `--mode=${process.features.debug ? 'debug' : 'release'}`,
-  `--shell=${process.execPath}`,
-  '--node-args=--no-async-context-frame',
-  '*/test-async-local-storage-*',
-], {
-  stdio: ['inherit', 'inherit', 'inherit'],
-});
-
-const [code] = await once(proc, 'exit');
-assert.strictEqual(code, 0);