diff --git a/bindings/binding.cc b/bindings/binding.cc index 68741dd8..ea46f95e 100644 --- a/bindings/binding.cc +++ b/bindings/binding.cc @@ -29,6 +29,40 @@ #include #endif +// Whether the isolate's ContinuationPreservedEmbedderData is a JS Map that +// currently binds `key` to `value`. +// +// This exists for AsyncContextFrame feature detection. With ACF active, Node +// implements AsyncLocalStorage#run by installing an AsyncContextFrame — a JS +// Map keyed by the AsyncLocalStorage instance — as the CPED of the running +// continuation. Calling this from inside a run() with the storage and its +// store therefore observes the property this addon actually depends on, +// instead of inferring it from the Node version, process.execArgv, or whether +// run() happens to dispatch through the instance's enterWith. +static NAN_METHOD(CpedMapContains) { +#if NODE_MAJOR_VERSION >= 22 + // A malformed call must not accidentally answer true by comparing an absent + // key's undefined against an undefined expected value. + if (info.Length() >= 2) { + auto isolate = info.GetIsolate(); + auto cped = isolate->GetContinuationPreservedEmbedderData(); + if (!cped.IsEmpty() && cped->IsMap()) { + auto context = isolate->GetCurrentContext(); + if (!context.IsEmpty()) { + v8::Local found; + if (cped.As()->Get(context, info[0]).ToLocal(&found)) { + info.GetReturnValue().Set(found->StrictEquals(info[1])); + return; + } + } + } + } +#endif + // Either code above didn't reach the innermost if statement, or + // we're compiling for Node.js < 22. + info.GetReturnValue().Set(false); +} + static NAN_METHOD(GetNativeThreadId) { #ifdef __APPLE__ uint64_t native_id; @@ -56,4 +90,5 @@ NODE_MODULE_INIT(/* exports, module, context */) { dd::WallProfiler::Init(exports); dd::OtelThreadCtx::Init(exports); Nan::SetMethod(exports, "getNativeThreadId", GetNativeThreadId); + Nan::SetMethod(exports, "cpedMapContains", CpedMapContains); } diff --git a/ts/src/async-context-frame.ts b/ts/src/async-context-frame.ts index fed0635f..a9d6041b 100644 --- a/ts/src/async-context-frame.ts +++ b/ts/src/async-context-frame.ts @@ -15,6 +15,23 @@ */ import {AsyncLocalStorage} from 'node:async_hooks'; +import {join} from 'path'; + +interface Addon { + cpedMapContains(key: unknown, value: unknown): boolean; +} + +let addon: Addon | undefined; + +// Required lazily so importing this module doesn't force the addon to load; +// memoized by isAsyncContextFrameActive, so this runs at most once per thread. +function bindings(): Addon { + if (!addon) { + const findBinding = require('node-gyp-build'); + addon = findBinding(join(__dirname, '..', '..')) as Addon; + } + return addon; +} let active: boolean | undefined; @@ -27,9 +44,10 @@ let active: boolean | undefined; * `process.execArgv`, because the two disagree in both directions and each * combination is reachable today: * - * - `NODE_OPTIONS=--experimental-async-context-frame` is accepted on Node 22 - * and 23 and turns ACF on without appearing in `execArgv`. Inferring "off" - * there makes callers refuse to run in a process that would have worked. + * - `NODE_OPTIONS=--experimental-async-context-frame` is accepted from Node + * 22.7.0 through 23 and turns ACF on without appearing in `execArgv`. + * Inferring "off" there makes callers refuse to run in a process that would + * have worked. * - `NODE_OPTIONS=--no-async-context-frame` is accepted on Node 24 and turns * ACF off without appearing in `execArgv`. Inferring "on" there is the worse * error: the CPED slot is never written, so a writer that starts anyway keeps @@ -39,19 +57,34 @@ let active: boolean | undefined; * main thread's command line either, and tooling sometimes rewrites * `process.execArgv` outright. * - * With ACF, `run()` is implemented in terms of `enterWith()`; without it, it - * isn't. Memoized: the answer is fixed for the life of the thread. + * Detected by asking the addon what is in the CPED slot during a `run()`. With + * ACF, Node installs an AsyncContextFrame — a JS Map keyed by the + * `AsyncLocalStorage` instance, valued by its store — as the running + * continuation's CPED; without it, nothing writes the slot. So a probe storage + * whose own store is visible there is direct evidence, and it is evidence about + * the exact slot both consumers read: `WallProfiler::SetContext` requires that + * Map, and the thread-ctx reader looks this very key up by the identity hash + * published as `als_identity_hash`. + * + * Observing whether `run()` delegates to `enterWith()` would be an indirect + * proxy for the same thing: it holds today, but it depends on `run()` + * dispatching through the instance property, which is unspecified and which + * anything patching `AsyncLocalStorage` can break — and the failure would be + * silent and in the dangerous direction. + * + * Memoized: the answer is fixed for the life of the thread. */ export function isAsyncContextFrameActive(): boolean { if (active === undefined) { - const probe = new AsyncLocalStorage(); - let delegated = false; - probe.enterWith = () => { - delegated = true; - }; - probe.run(0, () => {}); + const probe = new AsyncLocalStorage(); + // Object identity, so a stray equal-valued binding can't answer for us. + const sentinel = {}; + let bound = false; + probe.run(sentinel, () => { + bound = bindings().cpedMapContains(probe, sentinel); + }); probe.disable(); - active = delegated; + active = bound; } return active; } @@ -65,8 +98,10 @@ export function isAsyncContextFrameActive(): boolean { */ export function asyncContextFrameHint(): string { const version = process.versions.node; - const major = Number(version.split('.')[0]); - if (major < 22) { + const [major, minor] = version.split('.').map(Number); + // Hand-rolled rather than semver.satisfies: semver is a devDependency, and + // this module ships. + if (major < 22 || (major === 22 && minor < 7)) { return `Node ${version} does not support it at all; Node 24 and later enable it by default`; } if (major < 24) { diff --git a/ts/test/test-async-context-frame.ts b/ts/test/test-async-context-frame.ts index 4fe7e3de..54a0e970 100644 --- a/ts/test/test-async-context-frame.ts +++ b/ts/test/test-async-context-frame.ts @@ -15,14 +15,23 @@ */ import {strict as assert} from 'assert'; +import {AsyncLocalStorage} from 'node:async_hooks'; import {fork} from 'node:child_process'; import {join} from 'node:path'; +import {satisfies} from 'semver'; + import {isAsyncContextFrameActive} from '../src/async-context-frame'; +const addon = require('node-gyp-build')(join(__dirname, '..', '..')) as { + cpedMapContains(key?: unknown, value?: unknown): boolean; +}; + const CHILD = join(__dirname, 'async-context-frame-child.js'); const major = Number(process.versions.node.split('.')[0]); +// ACF landed in 22.7.0, so the opt-in routes are gated on that, not on major 22. +const hasAcfSupport = satisfies(process.versions.node, '>=22.7.0'); interface ChildReport { active: boolean; @@ -79,7 +88,7 @@ describe('isAsyncContextFrameActive', () => { }); it('reports it inactive when Node has no support for it', async function () { - if (major >= 22) return this.skip(); + if (hasAcfSupport) return this.skip(); const {active} = await probeChild(); assert.equal(active, false); }); @@ -107,11 +116,11 @@ describe('isAsyncContextFrameActive', () => { }); it('reports it active when NODE_OPTIONS turns it on', async function () { - // The mirror image, on the other Node line: 22 and 23 accept the flag in + // The mirror image, on the other Node line: 22.7.0 through 23 accept the flag in // NODE_OPTIONS (24 rejects it outright), again without it reaching execArgv, // so inferring from execArgv concludes ACF is off when it is on — and the // caller refuses to run in a process that would have worked. - if (major < 22 || major >= 24) return this.skip(); + if (!hasAcfSupport || major >= 24) return this.skip(); const {active, execArgv} = await probeChild({ nodeOptions: '--experimental-async-context-frame', }); @@ -119,3 +128,71 @@ describe('isAsyncContextFrameActive', () => { assert.equal(active, true); }); }); + +// The detection asks whether the running storage is bound to its own store, +// not merely whether the CPED slot holds a Map. These pin that difference: +// without them, weakening the helper to a bare IsMap check would still pass +// every test above. +describe('cpedMapContains', () => { + beforeEach(function () { + // With ACF off nothing writes the slot, so every answer here is false for + // an uninteresting reason. The routes that discriminate on/off are covered + // by the child-process cases above. + if (!isAsyncContextFrameActive()) this.skip(); + }); + + it('finds the running storage bound to its store', () => { + const als = new AsyncLocalStorage(); + const store = {}; + let found = false; + als.run(store, () => { + found = addon.cpedMapContains(als, store); + }); + als.disable(); + assert.equal(found, true); + }); + + it('does not match a foreign key', () => { + // CPED is a general embedder slot. Another native addon storing a Map there + // must not be able to answer for us, which is the false positive an IsMap + // check would admit. + const als = new AsyncLocalStorage(); + const store = {}; + let found = true; + als.run(store, () => { + found = addon.cpedMapContains(new AsyncLocalStorage(), store); + }); + als.disable(); + assert.equal(found, false); + }); + + it('does not match a different value for the right key', () => { + const als = new AsyncLocalStorage(); + let found = true; + als.run({}, () => { + found = addon.cpedMapContains(als, {}); + }); + als.disable(); + assert.equal(found, false); + }); + + it('is false outside any run', () => { + const als = new AsyncLocalStorage(); + const store = {}; + als.run(store, () => {}); + als.disable(); + assert.equal(addon.cpedMapContains(als, store), false); + }); + + it('is false when called without a key and value', () => { + // An absent key reads as undefined; so would a missing expected value, so + // a malformed call must not compare the two and report success. + const als = new AsyncLocalStorage(); + let found = true; + als.run({}, () => { + found = addon.cpedMapContains(); + }); + als.disable(); + assert.equal(found, false); + }); +}); diff --git a/ts/test/test-get-value-from-map-profiler.ts b/ts/test/test-get-value-from-map-profiler.ts index 1b926a25..6be2dc48 100644 --- a/ts/test/test-get-value-from-map-profiler.ts +++ b/ts/test/test-get-value-from-map-profiler.ts @@ -29,15 +29,13 @@ import assert from 'assert'; import {join} from 'path'; import {AsyncLocalStorage} from 'async_hooks'; -import {satisfies} from 'semver'; import {isAsyncContextFrameActive} from '../src/async-context-frame'; const findBinding = require('node-gyp-build'); const profiler = findBinding(join(__dirname, '..', '..')); -const useCPED = - isAsyncContextFrameActive() && satisfies(process.versions.node, '>=22.7.0'); +const useCPED = isAsyncContextFrameActive(); const supportedPlatform = process.platform === 'darwin' || process.platform === 'linux'; diff --git a/ts/test/test-otel-thread-ctx.ts b/ts/test/test-otel-thread-ctx.ts index bfb71118..e28a6f03 100644 --- a/ts/test/test-otel-thread-ctx.ts +++ b/ts/test/test-otel-thread-ctx.ts @@ -62,8 +62,9 @@ function tcIsTruncated(): boolean { } const isLinux = process.platform === 'linux'; -// AsyncContextFrame is the writer's discovery substrate: opt-in on Node 22/23 -// (via --experimental-async-context-frame) and on by default from Node 24 +// AsyncContextFrame is the writer's discovery substrate: opt-in from Node +// 22.7.0 through 23 (via --experimental-async-context-frame) and on by +// default from Node 24 // (disable-able via --no-async-context-frame). The TS layer refuses to install // the hook when it isn't active, so the entire describe block is skipped then. // Asks the same question the source side asks, the same way. diff --git a/ts/test/test-time-profiler.ts b/ts/test/test-time-profiler.ts index 0e249c7d..1738ee94 100644 --- a/ts/test/test-time-profiler.ts +++ b/ts/test/test-time-profiler.ts @@ -32,8 +32,7 @@ import {fork} from 'child_process'; import assert from 'assert'; -const useCPED = - isAsyncContextFrameActive() && satisfies(process.versions.node, '>=22.7.0'); +const useCPED = isAsyncContextFrameActive(); const collectAsyncId = satisfies(process.versions.node, '>=24.0.0'); diff --git a/ts/test/worker.ts b/ts/test/worker.ts index 5485196b..1da10334 100644 --- a/ts/test/worker.ts +++ b/ts/test/worker.ts @@ -12,10 +12,7 @@ const DURATION_MILLIS = 1000; const intervalMicros = 10000; const withContexts = process.platform === 'darwin' || process.platform === 'linux'; -const useCPED = - withContexts && - isAsyncContextFrameActive() && - satisfies(process.versions.node, '>=22.7.0'); +const useCPED = withContexts && isAsyncContextFrameActive(); const collectAsyncId = withContexts && satisfies(process.versions.node, '>=24.0.0'); diff --git a/ts/test/worker2.ts b/ts/test/worker2.ts index 0eed62c0..041a4461 100644 --- a/ts/test/worker2.ts +++ b/ts/test/worker2.ts @@ -10,10 +10,7 @@ const INTERVAL_MICROS = 10000; const withContexts = process.platform === 'darwin' || process.platform === 'linux'; -const useCPED = - withContexts && - isAsyncContextFrameActive() && - satisfies(process.versions.node, '>=22.7.0'); +const useCPED = withContexts && isAsyncContextFrameActive(); const collectAsyncId = withContexts && satisfies(process.versions.node, '>=24.0.0');