Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: improve async_context_frame structure #54239

Merged
merged 1 commit into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -894,8 +894,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
Expand Down Expand Up @@ -3472,6 +3472,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
[`Buffer`]: buffer.md#class-buffer
[`CRYPTO_secure_malloc_init`]: https://www.openssl.org/docs/man3.0/man3/CRYPTO_secure_malloc_init.html
[`NODE_OPTIONS`]: #node_optionsoptions
Expand Down
2 changes: 1 addition & 1 deletion lib/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
54 changes: 39 additions & 15 deletions lib/internal/async_context_frame.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,27 @@
'use strict';

const {
ObjectSetPrototypeOf,
} = primordials;

const {
getContinuationPreservedEmbedderData,
setContinuationPreservedEmbedderData,
} = internalBinding('async_context_frame');

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) {
Expand All @@ -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);
Flarna marked this conversation as resolved.
Show resolved Hide resolved
}

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);
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/process/task_queues.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Qard marked this conversation as resolved.
Show resolved Hide resolved

// *Must* match Environment::TickInfo::Fields in src/env.h.
const kHasTickScheduled = 0;
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,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;
Expand Down
2 changes: 1 addition & 1 deletion src/api/async_resource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ AsyncResource::AsyncResource(Isolate* isolate,

AsyncResource::~AsyncResource() {
CHECK_NOT_NULL(env_);
env_->RemoveAsyncResourceContextFrame(reinterpret_cast<std::uintptr_t>(this));
EmitAsyncDestroy(env_, async_context_);
env_->RemoveAsyncResourceContextFrame(reinterpret_cast<std::uintptr_t>(this));
}

MaybeLocal<Value> AsyncResource::MakeCallback(Local<Function> callback,
Expand Down
Loading