From b8970570b03ca2d0284641b021eb44d020824655 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Fri, 9 Aug 2024 12:44:42 -0700 Subject: [PATCH] lib: improve async_context_frame structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/54239 Reviewed-By: Gerhard Stöbich Reviewed-By: Chengzhong Wu --- doc/api/cli.md | 5 +- lib/async_hooks.js | 2 +- lib/internal/async_context_frame.js | 54 +++++++++++++------ .../{native.js => async_context_frame.js} | 0 lib/internal/process/task_queues.js | 2 +- lib/internal/timers.js | 2 +- src/api/async_resource.cc | 2 +- 7 files changed, 46 insertions(+), 21 deletions(-) rename lib/internal/async_local_storage/{native.js => async_context_frame.js} (100%) diff --git a/doc/api/cli.md b/doc/api/cli.md index 9f3e71eea45e8c..dfea34fe638280 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -884,8 +884,8 @@ added: REPLACEME > Stability: 1 - Experimental -Enables the use of AsyncLocalStorage backed by AsyncContextFrame rather than -the default implementation which relies on async\_hooks. This new model is +Enables the use of [`AsyncLocalStorage`][] backed by `AsyncContextFrame` rather +than the default implementation which relies on async\_hooks. This new model is implemented very differently and so could have differences in how context data flows within the application. As such, it is presently recommended to be sure your application behaviour is unaffected by this change before using it in @@ -3527,6 +3527,7 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12 [`--print`]: #-p---print-script [`--redirect-warnings`]: #--redirect-warningsfile [`--require`]: #-r---require-module +[`AsyncLocalStorage`]: async_context.md#class-asynclocalstorage [`Atomics.wait()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Atomics/wait [`Buffer`]: buffer.md#class-buffer [`CRYPTO_secure_malloc_init`]: https://www.openssl.org/docs/man3.0/man3/CRYPTO_secure_malloc_init.html diff --git a/lib/async_hooks.js b/lib/async_hooks.js index 27b1d3ebc3c4f0..f6d52a83fd004a 100644 --- a/lib/async_hooks.js +++ b/lib/async_hooks.js @@ -280,7 +280,7 @@ module.exports = { // Public API get AsyncLocalStorage() { return AsyncContextFrame.enabled ? - require('internal/async_local_storage/native') : + require('internal/async_local_storage/async_context_frame') : require('internal/async_local_storage/async_hooks'); }, createHook, diff --git a/lib/internal/async_context_frame.js b/lib/internal/async_context_frame.js index cd4f61b11554dc..fbf094e113375c 100644 --- a/lib/internal/async_context_frame.js +++ b/lib/internal/async_context_frame.js @@ -1,5 +1,9 @@ 'use strict'; +const { + ObjectSetPrototypeOf, +} = primordials; + const { getContinuationPreservedEmbedderData, setContinuationPreservedEmbedderData, @@ -7,28 +11,17 @@ const { let enabled_; -class AsyncContextFrame extends Map { - constructor(store, data) { - super(AsyncContextFrame.current()); - this.set(store, data); - } - +class ActiveAsyncContextFrame { static get enabled() { - enabled_ ??= require('internal/options') - .getOptionValue('--experimental-async-context-frame'); - return enabled_; + return true; } static current() { - if (this.enabled) { - return getContinuationPreservedEmbedderData(); - } + return getContinuationPreservedEmbedderData(); } static set(frame) { - if (this.enabled) { - setContinuationPreservedEmbedderData(frame); - } + setContinuationPreservedEmbedderData(frame); } static exchange(frame) { @@ -41,6 +34,37 @@ class AsyncContextFrame extends Map { const frame = this.current(); frame?.disable(store); } +} + +function checkEnabled() { + const enabled = require('internal/options') + .getOptionValue('--experimental-async-context-frame'); + + // If enabled, swap to active prototype so we don't need to check status + // on every interaction with the async context frame. + if (enabled) { + // eslint-disable-next-line no-use-before-define + ObjectSetPrototypeOf(AsyncContextFrame, ActiveAsyncContextFrame); + } + + return enabled; +} + +class AsyncContextFrame extends Map { + constructor(store, data) { + super(AsyncContextFrame.current()); + this.set(store, data); + } + + static get enabled() { + enabled_ ??= checkEnabled(); + return enabled_; + } + + static current() {} + static set(frame) {} + static exchange(frame) {} + static disable(store) {} disable(store) { this.delete(store); diff --git a/lib/internal/async_local_storage/native.js b/lib/internal/async_local_storage/async_context_frame.js similarity index 100% rename from lib/internal/async_local_storage/native.js rename to lib/internal/async_local_storage/async_context_frame.js diff --git a/lib/internal/process/task_queues.js b/lib/internal/process/task_queues.js index 3cec1000a0ca67..c7194977ba9db0 100644 --- a/lib/internal/process/task_queues.js +++ b/lib/internal/process/task_queues.js @@ -44,7 +44,7 @@ const { AsyncResource } = require('async_hooks'); const AsyncContextFrame = require('internal/async_context_frame'); -const async_context_frame = Symbol('asyncContextFrame'); +const async_context_frame = Symbol('kAsyncContextFrame'); // *Must* match Environment::TickInfo::Fields in src/env.h. const kHasTickScheduled = 0; diff --git a/lib/internal/timers.js b/lib/internal/timers.js index f2e79bcf2f30bc..3a78a0d40baae4 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -123,7 +123,7 @@ let debug = require('internal/util/debuglog').debuglog('timer', (fn) => { const AsyncContextFrame = require('internal/async_context_frame'); -const async_context_frame = Symbol('asyncContextFrame'); +const async_context_frame = Symbol('kAsyncContextFrame'); // *Must* match Environment::ImmediateInfo::Fields in src/env.h. const kCount = 0; diff --git a/src/api/async_resource.cc b/src/api/async_resource.cc index 9b826a35a28185..2ea5d1269875eb 100644 --- a/src/api/async_resource.cc +++ b/src/api/async_resource.cc @@ -27,8 +27,8 @@ AsyncResource::AsyncResource(Isolate* isolate, AsyncResource::~AsyncResource() { CHECK_NOT_NULL(env_); - env_->RemoveAsyncResourceContextFrame(reinterpret_cast(this)); EmitAsyncDestroy(env_, async_context_); + env_->RemoveAsyncResourceContextFrame(reinterpret_cast(this)); } MaybeLocal AsyncResource::MakeCallback(Local callback,