From 7f912c89068c982b81498c0b4910c72c5a6390a4 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Tue, 11 Aug 2026 13:55:52 +0200 Subject: [PATCH 1/6] docs(release): classify branch-diff false positives (#395) branch-diff matches commits rather than content, so it reports commits whose changes are already on v5.x. Two thirds of its output for v5.18.0 was noise. The skill previously said only "skip commits that would result in empty cherry-picks", which gives no way to tell those apart from real ones. Document the three classes actually observed: a. Commits subsumed by the squash-merged 5.14.2/5.14.3/5.14.4 releases. Enumerated per release; a closed set that will not grow. b. Dependabot bumps superseded by a later bump of the same package on v5.x. Cherry-picking one downgrades the branch. c. #154, the 6.0.0-pre bump on main, which must never reach a 5.x branch. Applying these to `branch-diff v5.x main` yields exactly the 13 commits in the v5.18.0 proposal. Also add a `git diff --stat main` check before the version bump. An age-based cutoff had dismissed #352 as a false positive when it was real and unapplied; the content diff is what exposed it, so the skill now states that age alone is not evidence and cites #352 as the counterexample. Smaller fixes for things that misled during v5.18.0: pull both branches before comparing, parse PR numbers from the trailing URL rather than the "(#NNN)" form (which false-matches PR references in commit titles), clear the previous release's worktree, and keep the version commit last on the branch. --- .claude/skills/release/SKILL.md | 78 ++++++++++++++++++++++++++++++--- 1 file changed, 73 insertions(+), 5 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index b96de691..2ff29356 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -15,6 +15,13 @@ The `branch-diff` tool must be installed globally: npm install branch-diff -g ``` +Fetch and fast-forward **both** branches before doing anything else. Comparing a +stale `v5.x` against a stale `main` silently produces a wrong commit list: + +``` +git fetch origin && git checkout v5.x && git pull && git checkout main && git pull +``` + ## Steps ### 1. Identify commits to cherry-pick @@ -25,9 +32,36 @@ Use the `branch-diff` tool to list commits on `main` not yet applied to `v5.x`: branch-diff v5.x main ``` -Review the output with the user. Skip: -- Version bump commits (e.g. "Bump package version on to 6.0.0-pre") -- Commits that would result in empty cherry-picks (already applied or superseded) +Its GitHub issue-lookup errors go to stderr; the commit list is on stdout. PR numbers +appear in the trailing URL (`.../pull/393`), *not* as `(#393)` — parsing the `(#NNN)` +form instead picks up PR references that happen to appear in commit titles. + +`branch-diff` matches commits, not content, so it reports a substantial number of +**false positives** — commits whose changes are already on `v5.x`. Do not cherry-pick +these. They fall into three classes: + +**a. Squash-merged releases.** Releases 5.14.2, 5.14.3 and 5.14.4 were squash-merged +rather than rebased, so every commit they contained lost its identity and is reported +forever. This set is closed and will not grow — treat all of these as already released: + +| Release | Proposal | PRs subsumed | +|---|---|---| +| 5.14.2 | #331 | 284, 310, 311, 315, 316, 317, 320, 323, 324, 325, 326, 327, 329 | +| 5.14.3 | #334 | 328, 332 | +| 5.14.4 | #337 | 333, 335, 336 | + +**b. Superseded dependency bumps.** A Dependabot bump that never landed on `v5.x`, which +later picked up an equal-or-newer version of the same package directly. Cherry-picking one +would *downgrade* the branch. Recognise these by comparing the package version in +`v5.x:package.json` against the bump's target — skip when `v5.x` is at or ahead of it. +(Examples seen so far: #140, #344, #348, #349, #350.) + +**c. The `main`-only version bump.** #154 moved `main` to `6.0.0-pre`. It must never be +cherry-picked onto a 5.x release branch. + +Anything left after removing those three classes is a genuine candidate. Note that being +old is *not* by itself evidence of a false positive: #352 sat below all of these and was a +real, unapplied commit. Classify by the rules above, not by age. Confirm the list of commits with the user before proceeding. @@ -54,6 +88,13 @@ Create a git worktree from the current repo, checking out a new branch `v$VERSIO git worktree add ../pprof-nodejs-v5 -b v$VERSION-proposal v5.x ``` +The path is usually still occupied by the previous release's worktree. Once that +proposal's PR is merged, it is safe to clear — verify it is clean and merged first, then: + +``` +git worktree remove ../pprof-nodejs-v5 && git branch -D v-proposal +``` + All subsequent steps run in the worktree directory. ### 4. Cherry-pick commits @@ -66,7 +107,30 @@ git cherry-pick ... If a cherry-pick has conflicts, stop and resolve with the user. -### 5. Create the version bump commit +### 5. Verify the selection against `main` + +Before bumping the version, diff the worktree against `main`: + +``` +git diff --stat main -- . +``` + +The goal is **minimal divergence**: ideally this reports nothing but `package.json` and +`package-lock.json` (the version, plus any dev-dep bump this release includes). + +This is the check that validates step 1, and it is worth doing carefully — it is how #352 +was caught, a genuinely unapplied commit that a plausible-looking age heuristic had +written off as a false positive. Any *other* file appearing here means one of two things: + +- a real commit was wrongly classified as a false positive — cherry-pick it, or +- the divergence is deliberate — say so explicitly in the PR body rather than leaving it + silently unexplained. + +Note that a class-(b) superseded bump correctly shows up as a `package.json` / +`package-lock.json` difference where `v5.x` is *ahead* of `main`. That is expected and +should be left alone. + +### 6. Create the version bump commit Bump the version in package.json and package-lock.json using npm, then commit: @@ -76,7 +140,11 @@ git add package.json package-lock.json git commit -m "v$VERSION" ``` -### 6. Push and create a PR +Keep this commit last on the branch. If a further cherry-pick turns out to be needed after +this point, drop the version commit (`git reset --hard HEAD~1`), apply the cherry-pick, +then re-run the bump — rather than stacking the new commit on top of the release commit. + +### 7. Push and create a PR Push the branch and create a PR targeting `v5.x`: From 20b249d43f02993ad1fe17a2c46cd3e3db407ecf Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Thu, 13 Aug 2026 09:49:51 +0200 Subject: [PATCH 2/6] fix(otel-thread-ctx): feature-detect AsyncContextFrame (#397) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(otel-thread-ctx): feature-detect AsyncContextFrame The writer inferred whether AsyncContextFrame was available from the Node version plus `process.execArgv`, and threw from `enter()` when it concluded it was not. That inference is wrong in both directions, and each way is reachable with a flag Node itself accepts: # Node 22.23.2 — ACF on, execArgv empty: inference says "unavailable" $ NODE_OPTIONS=--experimental-async-context-frame node probe.js {"isACFActive":true,"execArgv":[]} # Node 24.18.0 — ACF off, execArgv empty: inference says "available" $ NODE_OPTIONS=--no-async-context-frame node probe.js {"isACFActive":false,"execArgv":[]} Node 22 and 23 accept --experimental-async-context-frame in NODE_OPTIONS (Node 24 rejects it, and does not need it); Node 24 accepts --no-async-context-frame there (Node 22 has no such flag). Neither reaches execArgv. A worker thread created with an explicit execArgv doesn't inherit the main thread's command line either, and tooling sometimes rewrites process.execArgv outright. The false-negative makes the writer refuse to run in a process where it would have worked. The false-positive is worse and silent: the CPED slot the addon reads is only written when ACF is on, so the writer installs its hook, keeps looking healthy from JS — getStore() still works — and every out-of-process reader sees a record that nothing ever updates. Ask the question directly instead: with ACF, AsyncLocalStorage#run is implemented in terms of #enterWith, and without it, it isn't. The version and execArgv are still used, but only to word the error message. Five test-side copies of the same inference decided whether to exercise the CPED paths, so they mis-skipped in exactly the same processes; they now share the one detection. Their >=22.7.0 floor for time-profiler CPED support is unchanged. * test(docker): stage the tree without tsconfig.tsbuildinfo The runner deletes the host's node_modules, build and out before building inside the container, but copies in tsconfig.tsbuildinfo, which is gitignored and present on any host where `npm run compile` has been run. tsc then trusts that incremental state, emits nothing for the deleted out/, and the run ends in Error: No test files found: "out/test/test-*.js" having tested nothing at all. --- bindings/otel-thread-ctx.cc | 2 +- scripts/docker/run-in-docker.sh | 6 +- ts/src/async-context-frame.ts | 76 ++++++++++++ ts/src/otel-thread-ctx.ts | 31 +++-- ts/test/async-context-frame-child.ts | 26 +++++ ts/test/test-async-context-frame.ts | 121 ++++++++++++++++++++ ts/test/test-get-value-from-map-profiler.ts | 7 +- ts/test/test-otel-thread-ctx.ts | 22 ++-- ts/test/test-time-profiler.ts | 6 +- ts/test/worker.ts | 7 +- ts/test/worker2.ts | 7 +- 11 files changed, 260 insertions(+), 51 deletions(-) create mode 100644 ts/src/async-context-frame.ts create mode 100644 ts/test/async-context-frame-child.ts create mode 100644 ts/test/test-async-context-frame.ts diff --git a/bindings/otel-thread-ctx.cc b/bindings/otel-thread-ctx.cc index cbe34d51..3d061891 100644 --- a/bindings/otel-thread-ctx.cc +++ b/bindings/otel-thread-ctx.cc @@ -756,7 +756,7 @@ void StoreAls(const FunctionCallbackInfo& args) { #else // Node < 22 lacks ContinuationPreservedEmbedderData entirely (and the // associated V8 internal offset). The TS layer refuses to install the - // hook on these versions via asyncContextFrameError, so StoreAls is + // hook on these versions via isAsyncContextFrameActive, so StoreAls is // never called from JS — this null assignment is just here so the // addon compiles on the older Node versions the package supports. otel_thread_ctx_nodejs_v1.cped_slot = nullptr; diff --git a/scripts/docker/run-in-docker.sh b/scripts/docker/run-in-docker.sh index 099549da..7772cd83 100755 --- a/scripts/docker/run-in-docker.sh +++ b/scripts/docker/run-in-docker.sh @@ -31,7 +31,11 @@ exec docker run --rm \ set -euo pipefail cp -R /work/. /tmp/work/ # Drop any host-built artifacts so we get a clean build inside. - rm -rf /tmp/work/node_modules /tmp/work/build /tmp/work/out + # tsconfig.tsbuildinfo has to go with out/: left behind, tsc trusts it, + # emits nothing for the deleted out/, and the run ends in "No test files + # found" having tested nothing. + rm -rf /tmp/work/node_modules /tmp/work/build /tmp/work/out \ + /tmp/work/tsconfig.tsbuildinfo npm install --no-audit --no-fund npm test ' diff --git a/ts/src/async-context-frame.ts b/ts/src/async-context-frame.ts new file mode 100644 index 00000000..fed0635f --- /dev/null +++ b/ts/src/async-context-frame.ts @@ -0,0 +1,76 @@ +/* + * Copyright 2026 Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {AsyncLocalStorage} from 'node:async_hooks'; + +let active: boolean | undefined; + +/** + * Whether this process's `AsyncLocalStorage` is backed by AsyncContextFrame, + * which is what puts the active value in the isolate's + * ContinuationPreservedEmbedderData slot that this addon reads. + * + * Feature-detected rather than inferred from the Node version plus + * `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=--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 + * looking healthy from JS — `getStore()` still works — while every + * out-of-process reader sees a record that nothing ever updates. + * - A worker thread created with an explicit `execArgv` doesn't inherit the + * 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. + */ +export function isAsyncContextFrameActive(): boolean { + if (active === undefined) { + const probe = new AsyncLocalStorage(); + let delegated = false; + probe.enterWith = () => { + delegated = true; + }; + probe.run(0, () => {}); + probe.disable(); + active = delegated; + } + return active; +} + +/** + * How to turn AsyncContextFrame on, for the error message of whatever declined + * to run without it. + * + * Advisory text only — never decide availability from this. That is what + * {@link isAsyncContextFrameActive} is for. + */ +export function asyncContextFrameHint(): string { + const version = process.versions.node; + const major = Number(version.split('.')[0]); + if (major < 22) { + return `Node ${version} does not support it at all; Node 24 and later enable it by default`; + } + if (major < 24) { + return `Node ${version} needs --experimental-async-context-frame, on the command line or in NODE_OPTIONS; Node 24 and later enable it by default`; + } + return `Node ${version} enables it by default, so something turned it off — look for --no-async-context-frame on the command line, in NODE_OPTIONS, or in this worker's execArgv`; +} diff --git a/ts/src/otel-thread-ctx.ts b/ts/src/otel-thread-ctx.ts index f59a976b..6ee061ed 100644 --- a/ts/src/otel-thread-ctx.ts +++ b/ts/src/otel-thread-ctx.ts @@ -19,6 +19,11 @@ // as a near-verbatim copy: edits should ideally land upstream first and // be ported here, so the two stay in sync. We plan to drop this vendored // copy once the upstream package is suitable to depend on directly. +// +// Known divergence from upstream: AsyncContextFrame availability is +// feature-detected via ./async-context-frame instead of being inferred from +// `process.execArgv`, which is wrong in both directions — see that module. Keep +// the divergence across re-syncs until upstream does the same. // Node.js writer for the OpenTelemetry Thread Local Context Record // (OTEP-4947), discoverable from an out-of-process reader via the @@ -30,6 +35,11 @@ import {join} from 'path'; import {AsyncLocalStorage} from 'node:async_hooks'; +import { + asyncContextFrameHint, + isAsyncContextFrameActive, +} from './async-context-frame'; + /** * OTEP-4719 process-context attributes corresponding to a particular * key list. Spread this into whatever attribute map the application @@ -171,27 +181,12 @@ if (process.platform === 'linux') { let als: AsyncLocalStorage | undefined; - function asyncContextFrameError(): string | undefined { - const [major] = process.versions.node.split('.').map(Number); - if (process.execArgv.includes('--no-async-context-frame')) { - return 'Node explicitly launched with --no-async-context-frame'; - } - if (major >= 24) return undefined; - if (process.execArgv.includes('--experimental-async-context-frame')) { - return undefined; - } - if (major >= 22) { - return 'Node versions prior to v24 must be launched with --experimental-async-context-frame'; - } - return 'Node major versions prior to v22 do not support the feature at all'; - } - function ensureHook(): AsyncLocalStorage { if (als) return als; - const err = asyncContextFrameError(); - if (err) { + if (!isAsyncContextFrameActive()) { throw new Error( - `otel thread-ctx writer requires async_context_frame support, which is unavailable: ${err}.`, + 'otel thread-ctx writer requires async_context_frame support, which is ' + + `unavailable: ${asyncContextFrameHint()}.`, ); } als = new AsyncLocalStorage(); diff --git a/ts/test/async-context-frame-child.ts b/ts/test/async-context-frame-child.ts new file mode 100644 index 00000000..ededfbe9 --- /dev/null +++ b/ts/test/async-context-frame-child.ts @@ -0,0 +1,26 @@ +/* + * Copyright 2026 Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Reports how this process sees AsyncContextFrame, for test-async-context-frame +// to compare against how the flags reached it. Also reports execArgv, so a +// failure shows whether the flag was visible there at all. + +import {isAsyncContextFrameActive} from '../src/async-context-frame'; + +process.send?.({ + active: isAsyncContextFrameActive(), + execArgv: process.execArgv, +}); diff --git a/ts/test/test-async-context-frame.ts b/ts/test/test-async-context-frame.ts new file mode 100644 index 00000000..4fe7e3de --- /dev/null +++ b/ts/test/test-async-context-frame.ts @@ -0,0 +1,121 @@ +/* + * Copyright 2026 Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {strict as assert} from 'assert'; +import {fork} from 'node:child_process'; +import {join} from 'node:path'; + +import {isAsyncContextFrameActive} from '../src/async-context-frame'; + +const CHILD = join(__dirname, 'async-context-frame-child.js'); + +const major = Number(process.versions.node.split('.')[0]); + +interface ChildReport { + active: boolean; + execArgv: string[]; +} + +// Runs the probe in a child process configured the way the test wants, since +// AsyncContextFrame is decided at process start and can't be toggled in-process. +function probeChild( + options: {execArgv?: string[]; nodeOptions?: string} = {}, +): Promise { + return new Promise((resolve, reject) => { + const child = fork(CHILD, [], { + execArgv: options.execArgv ?? [], + env: options.nodeOptions + ? {...process.env, NODE_OPTIONS: options.nodeOptions} + : {...process.env, NODE_OPTIONS: ''}, + stdio: ['ignore', 'ignore', 'pipe', 'ipc'], + }); + let report: ChildReport | undefined; + let stderr = ''; + child.stderr?.on('data', chunk => { + stderr += chunk; + }); + child.on('message', message => { + report = message as ChildReport; + }); + child.on('error', reject); + child.on('exit', code => { + if (report === undefined) { + reject( + new Error( + `child exited with ${code} and no report; stderr: ${stderr}`, + ), + ); + return; + } + resolve(report); + }); + }); +} + +describe('isAsyncContextFrameActive', () => { + it('gives the same answer on every call', () => { + const first = isAsyncContextFrameActive(); + assert.equal(typeof first, 'boolean'); + assert.equal(isAsyncContextFrameActive(), first); + }); + + it('reports it active when Node enables it by default', async function () { + if (major < 24) return this.skip(); + const {active} = await probeChild(); + assert.equal(active, true); + }); + + it('reports it inactive when Node has no support for it', async function () { + if (major >= 22) return this.skip(); + const {active} = await probeChild(); + assert.equal(active, false); + }); + + it('reports it inactive when the command line turns it off', async function () { + // The flag only exists from Node 24, where ACF is the default. + if (major < 24) return this.skip(); + const {active} = await probeChild({ + execArgv: ['--no-async-context-frame'], + }); + assert.equal(active, false); + }); + + it('reports it inactive when NODE_OPTIONS turns it off', async function () { + // The regression this detection exists for: Node 24 accepts the flag in + // NODE_OPTIONS, where it does not reach execArgv, so inferring from execArgv + // concludes ACF is on. It is off, the CPED slot is never written, and a + // caller that trusted the inference would emit records nothing updates. + if (major < 24) return this.skip(); + const {active, execArgv} = await probeChild({ + nodeOptions: '--no-async-context-frame', + }); + assert.deepEqual(execArgv, []); + assert.equal(active, false); + }); + + 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 + // 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(); + const {active, execArgv} = await probeChild({ + nodeOptions: '--experimental-async-context-frame', + }); + assert.deepEqual(execArgv, []); + assert.equal(active, true); + }); +}); diff --git a/ts/test/test-get-value-from-map-profiler.ts b/ts/test/test-get-value-from-map-profiler.ts index 432dac5c..1b926a25 100644 --- a/ts/test/test-get-value-from-map-profiler.ts +++ b/ts/test/test-get-value-from-map-profiler.ts @@ -31,14 +31,13 @@ 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 = - (satisfies(process.versions.node, '>=24.0.0') && - !process.execArgv.includes('--no-async-context-frame')) || - (satisfies(process.versions.node, '>=22.7.0') && - process.execArgv.includes('--experimental-async-context-frame')); + isAsyncContextFrameActive() && satisfies(process.versions.node, '>=22.7.0'); 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 f4d6683b..bfb71118 100644 --- a/ts/test/test-otel-thread-ctx.ts +++ b/ts/test/test-otel-thread-ctx.ts @@ -30,6 +30,7 @@ import {fork, spawnSync} from 'node:child_process'; import {existsSync} from 'node:fs'; import {join} from 'node:path'; +import {isAsyncContextFrameActive} from '../src/async-context-frame'; import { ThreadContext, getContext, @@ -61,21 +62,12 @@ function tcIsTruncated(): boolean { } const isLinux = process.platform === 'linux'; -// AsyncContextFrame (the writer's discovery substrate) is opt-in on Node -// 22/23 (via --experimental-async-context-frame) and on by default in -// Node 24+ (disable-able via --no-async-context-frame). The TS layer -// refuses to install the hook when ACF isn't available, so the entire -// describe block is skipped in that case. Mirrors the source-side -// asyncContextFrameError logic. -const isAsyncContextFrameAvailable = (() => { - if (process.execArgv.includes('--no-async-context-frame')) return false; - const major = Number(process.versions.node.split('.')[0]); - if (major >= 24) return true; - if (major >= 22) { - return process.execArgv.includes('--experimental-async-context-frame'); - } - return false; -})(); +// 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 +// (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. +const isAsyncContextFrameAvailable = isAsyncContextFrameActive(); // Returns a plain Uint8Array (not a Buffer) so assert.deepStrictEqual against // other Uint8Arrays — including the one the addon returns — succeeds. diff --git a/ts/test/test-time-profiler.ts b/ts/test/test-time-profiler.ts index ede45f7a..0e249c7d 100644 --- a/ts/test/test-time-profiler.ts +++ b/ts/test/test-time-profiler.ts @@ -15,6 +15,7 @@ */ import * as sinon from 'sinon'; +import {isAsyncContextFrameActive} from '../src/async-context-frame'; import {time, getNativeThreadId} from '../src'; import {profileV2, stopV2} from '../src/time-profiler'; import * as v8TimeProfiler from '../src/time-profiler-bindings'; @@ -32,10 +33,7 @@ import {fork} from 'child_process'; import assert from 'assert'; const useCPED = - (satisfies(process.versions.node, '>=24.0.0') && - !process.execArgv.includes('--no-async-context-frame')) || - (satisfies(process.versions.node, '>=22.7.0') && - process.execArgv.includes('--experimental-async-context-frame')); + isAsyncContextFrameActive() && satisfies(process.versions.node, '>=22.7.0'); const collectAsyncId = satisfies(process.versions.node, '>=24.0.0'); diff --git a/ts/test/worker.ts b/ts/test/worker.ts index 5b4240af..5485196b 100644 --- a/ts/test/worker.ts +++ b/ts/test/worker.ts @@ -4,6 +4,7 @@ import {time} from '../src/index'; import {Profile, ValueType} from 'pprof-format'; import {getAndVerifyPresence, getAndVerifyString} from './profiles-for-tests'; import {satisfies} from 'semver'; +import {isAsyncContextFrameActive} from '../src/async-context-frame'; import assert from 'assert'; @@ -13,10 +14,8 @@ const withContexts = process.platform === 'darwin' || process.platform === 'linux'; const useCPED = withContexts && - ((satisfies(process.versions.node, '>=24.0.0') && - !process.execArgv.includes('--no-async-context-frame')) || - (satisfies(process.versions.node, '>=22.7.0') && - process.execArgv.includes('--experimental-async-context-frame'))); + isAsyncContextFrameActive() && + satisfies(process.versions.node, '>=22.7.0'); const collectAsyncId = withContexts && satisfies(process.versions.node, '>=24.0.0'); diff --git a/ts/test/worker2.ts b/ts/test/worker2.ts index 2a1e4b13..0eed62c0 100644 --- a/ts/test/worker2.ts +++ b/ts/test/worker2.ts @@ -1,6 +1,7 @@ import {parentPort} from 'node:worker_threads'; import {time} from '../src/index'; import {satisfies} from 'semver'; +import {isAsyncContextFrameActive} from '../src/async-context-frame'; const delay = (ms: number) => new Promise(res => setTimeout(res, ms)); @@ -11,10 +12,8 @@ const withContexts = const useCPED = withContexts && - ((satisfies(process.versions.node, '>=24.0.0') && - !process.execArgv.includes('--no-async-context-frame')) || - (satisfies(process.versions.node, '>=22.7.0') && - process.execArgv.includes('--experimental-async-context-frame'))); + isAsyncContextFrameActive() && + satisfies(process.versions.node, '>=22.7.0'); const collectAsyncId = withContexts && satisfies(process.versions.node, '>=24.0.0'); From bf1f203dfc1535bfe1a4b4c4612627f1e8008cb9 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Fri, 14 Aug 2026 13:51:03 +0200 Subject: [PATCH 3/6] fix(otel-thread-ctx): detect AsyncContextFrame by reading CPED natively (#398) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(otel-thread-ctx): detect AsyncContextFrame by reading CPED natively #397 replaced the execArgv inference with a feature detection, but the probe was indirect: it overrode `enterWith` on a throwaway AsyncLocalStorage and checked whether `run()` dispatched through it. That `run()` goes through the instance property is unspecified, and anything patching AsyncLocalStorage can break it — including dd-trace-js, which patches async-context machinery. The resulting false negative is the failure #397 set out to fix: `enter()` throwing inside a diagnostic-channel subscriber, in application code. Ask the question directly instead. `cpedMapContains(key, value)` reports whether the isolate's ContinuationPreservedEmbedderData binds a key to a value, so calling it from inside a `run()` with the probe storage and its own store observes the property the addon actually depends on. It is the same slot, and the same "is it a Map" question, that WallProfiler::SetContext asks before storing a context; the key is the one whose identity hash is published as otel_thread_ctx_nodejs_v1.als_identity_hash for the out-of-process reader to look up. Verified empirically that the frame is keyed by the storage instance with the store as value. Checking the key and value rather than just "CPED holds a Map" matters: CPED is a general embedder slot, so a Map another addon left there must not answer for us — that would resurrect the silent false positive, where the writer looks healthy from JS while readers see records nothing updates. * test: use the real 22.7.0 AsyncContextFrame cutoff --- bindings/binding.cc | 35 +++++++++ ts/src/async-context-frame.ts | 63 ++++++++++++---- ts/test/test-async-context-frame.ts | 83 ++++++++++++++++++++- ts/test/test-get-value-from-map-profiler.ts | 4 +- ts/test/test-otel-thread-ctx.ts | 5 +- ts/test/test-time-profiler.ts | 3 +- ts/test/worker.ts | 5 +- ts/test/worker2.ts | 5 +- 8 files changed, 171 insertions(+), 32 deletions(-) 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'); From 4b0a843889094d208768a45fff76203fe776e6c2 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:51:42 +0000 Subject: [PATCH 4/6] build(deps-dev): bump the minor-updates group with 4 updates (#396) Bumps the minor-updates group with 4 updates: [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node), [@types/semver](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/semver), [eslint-plugin-n](https://github.com/eslint-community/eslint-plugin-n) and [mocha](https://github.com/mochajs/mocha). Updates `@types/node` from 26.1.2 to 26.2.0 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) Updates `@types/semver` from 7.7.1 to 7.8.0 - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/semver) Updates `eslint-plugin-n` from 18.2.2 to 18.3.0 - [Release notes](https://github.com/eslint-community/eslint-plugin-n/releases) - [Changelog](https://github.com/eslint-community/eslint-plugin-n/blob/master/CHANGELOG.md) - [Commits](https://github.com/eslint-community/eslint-plugin-n/compare/v18.2.2...v18.3.0) Updates `mocha` from 11.7.6 to 11.8.0 - [Release notes](https://github.com/mochajs/mocha/releases) - [Changelog](https://github.com/mochajs/mocha/blob/v11.8.0/CHANGELOG.md) - [Commits](https://github.com/mochajs/mocha/compare/v11.7.6...v11.8.0) --- updated-dependencies: - dependency-name: "@types/node" dependency-version: 26.2.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: minor-updates - dependency-name: "@types/semver" dependency-version: 7.8.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: minor-updates - dependency-name: eslint-plugin-n dependency-version: 18.3.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: minor-updates - dependency-name: mocha dependency-version: 11.8.0 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: minor-updates ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 32 ++++++++++++++++---------------- package.json | 8 ++++---- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/package-lock.json b/package-lock.json index bbfc19ac..becc7333 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,17 +15,17 @@ }, "devDependencies": { "@types/mocha": "^10.0.1", - "@types/node": "26.1.2", - "@types/semver": "^7.5.8", + "@types/node": "26.2.0", + "@types/semver": "^7.8.0", "@types/sinon": "^22.0.0", "@types/tmp": "^0.2.3", "clang-format": "^1.8.0", "codecov": "^3.8.3", "deep-copy": "^1.4.2", - "eslint-plugin-n": "^18.2.2", + "eslint-plugin-n": "^18.3.0", "gts": "^7.0.0", "js-green-licenses": "^4.0.0", - "mocha": "^11.7.6", + "mocha": "^11.8.0", "nan": "^2.28.0", "nyc": "^18.0.0", "semver": "^7.8.5", @@ -962,9 +962,9 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "26.1.2", - "resolved": "https://registry.npmjs.org/@types/node/-/node-26.1.2.tgz", - "integrity": "sha512-Vu4a5UFA9rIIFJ7rB/Vaafh9lrCQszopTCx6KjFboXTGQbPNasehVR5TEiithSDGyd1DEiUByggTZsg8jukeIg==", + "version": "26.2.0", + "resolved": "https://registry.npmjs.org/@types/node/-/node-26.2.0.tgz", + "integrity": "sha512-5IviulTZeRNp2vAJ514cc/HUlY5nZ9fCbq9DMyC52BrhFZACo3nI0R7qBxhQmo/d27NFe96ur/b7Wwxklda+kg==", "dev": true, "license": "MIT", "dependencies": { @@ -989,9 +989,9 @@ } }, "node_modules/@types/semver": { - "version": "7.7.1", - "resolved": "https://registry.npmjs.org/@types/semver/-/semver-7.7.1.tgz", - "integrity": "sha512-FmgJfu+MOcQ370SD0ev7EI8TlCAfKYU+B4m5T3yXc1CiRN94g/SZPtsCkk506aUDtlMnFZvasDwHHUcZUEaYuA==", + "version": "7.8.0", + "resolved": "https://registry.npmjs.org/@types/semver/-/semver-7.8.0.tgz", + "integrity": "sha512-1mAINjtQCXXeLkJ9ehXkwOcBpqtLxiVtKhpUf83DdRNdQKV0iXZpaHYqRr7nj+wvxuJzoAmAwXI+sCNMv1CzLQ==", "dev": true, "license": "MIT" }, @@ -2245,9 +2245,9 @@ } }, "node_modules/eslint-plugin-n": { - "version": "18.2.2", - "resolved": "https://registry.npmjs.org/eslint-plugin-n/-/eslint-plugin-n-18.2.2.tgz", - "integrity": "sha512-gOO0lIqwEjZ750kv9/SptCWArUoAZXJoBr0vYWTO2dCBxctHUXlBIigiC8xuxxr/NKqgIT6Ehz1xRcilj8a5cA==", + "version": "18.3.0", + "resolved": "https://registry.npmjs.org/eslint-plugin-n/-/eslint-plugin-n-18.3.0.tgz", + "integrity": "sha512-cPVguuDe6DrIPb/qUXHf8P89MaVTUmiYWwpt5gX5AILsvRIiZAxMFXcFR6QHYBksqKJpjfUBlL/RleCJUWcD7w==", "dev": true, "license": "MIT", "dependencies": { @@ -4105,9 +4105,9 @@ } }, "node_modules/mocha": { - "version": "11.7.6", - "resolved": "https://registry.npmjs.org/mocha/-/mocha-11.7.6.tgz", - "integrity": "sha512-nS9xOGbw2I3cjCpxwZAEJ9xK9lmJ08vEkQvLtz4du9ZrF9UrjRpeJGiIgl2Z+Qs++pmB4ecDe48Fwsh+j+j7xA==", + "version": "11.8.0", + "resolved": "https://registry.npmjs.org/mocha/-/mocha-11.8.0.tgz", + "integrity": "sha512-VyCeUdGN3A9lmCTTgG4yuvY9ixxaDk+xt2R/7/+1AP6EqNG+G9OKkzBwhVtVYoNX8YsxNSgAl8mOv3IAeOpFbw==", "dev": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 9092580a..e57b68a0 100644 --- a/package.json +++ b/package.json @@ -43,17 +43,17 @@ }, "devDependencies": { "@types/mocha": "^10.0.1", - "@types/node": "26.1.2", - "@types/semver": "^7.5.8", + "@types/node": "26.2.0", + "@types/semver": "^7.8.0", "@types/sinon": "^22.0.0", "@types/tmp": "^0.2.3", "clang-format": "^1.8.0", "codecov": "^3.8.3", "deep-copy": "^1.4.2", - "eslint-plugin-n": "^18.2.2", + "eslint-plugin-n": "^18.3.0", "gts": "^7.0.0", "js-green-licenses": "^4.0.0", - "mocha": "^11.7.6", + "mocha": "^11.8.0", "nan": "^2.28.0", "nyc": "^18.0.0", "semver": "^7.8.5", From e790e89bdcfb0594bd4e4ab264fa866b82789ec0 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Thu, 20 Aug 2026 16:51:31 +0200 Subject: [PATCH 5/6] fix(wall): don't touch V8 handles in ~WallProfiler (#399) --- bindings/otel-thread-ctx.cc | 5 +++++ bindings/profilers/wall.cc | 12 ++++++++---- ts/test/worker2.ts | 22 ++++++++++++++++++---- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/bindings/otel-thread-ctx.cc b/bindings/otel-thread-ctx.cc index 3d061891..ddee80cf 100644 --- a/bindings/otel-thread-ctx.cc +++ b/bindings/otel-thread-ctx.cc @@ -301,7 +301,12 @@ thread_local CtxWrap* g_live_ctx_wraps = nullptr; // fires exactly once, at teardown, while the Environment is still alive. void DrainLiveCtxWraps(void* arg) { auto* isolate = static_cast(arg); + // We must allocate our own HandleScope here as node::FreeEnvironment wraps + // RunCleanup in a SealHandleScope, so handle_.Get() below has to allocate + // inside a scope of our own or V8 aborts with "Cannot create a handle without + // a HandleScope". v8::HandleScope scope(isolate); + CtxWrap* p = g_live_ctx_wraps; while (p != nullptr) { CtxWrap* next = p->next_; diff --git a/bindings/profilers/wall.cc b/bindings/profilers/wall.cc index fe85c876..0187af47 100644 --- a/bindings/profilers/wall.cc +++ b/bindings/profilers/wall.cc @@ -700,15 +700,19 @@ WallProfiler::~WallProfiler() { // unlink. (~PCP still resets its weak handle during delete, so the dangling // internal-field pointer in the wrap object stays inert even if V8 later // GCs the wrap.) + // + // While it'd be tempting to do the same "zero out internal field logic" here + // as in otel-thread-ctx.cc's DrainLiveCtxWraps, we shouldn't. That one only + // ever runs as an environment cleanup hook, while this can also get here from + // Nan::ObjectWrap's weak callback, and V8 forbids the API in a first-pass + // weak callback. The holders' internal fields therefore keep pointing at the + // PCPs we free, but since they are only ever read back through our own + // cpedKey_ that dies with us it is not an issue. auto* p = liveContextPtrHead_; - auto isolate = Isolate::GetCurrent(); while (p != nullptr) { auto* next = p->next_; p->pprev_ = nullptr; p->next_ = nullptr; - if (isolate != nullptr && !p->handle_.IsEmpty()) { - SetAlignedPointerInInternalField(p->handle_.Get(isolate), 0, nullptr); - } delete p; p = next; } diff --git a/ts/test/worker2.ts b/ts/test/worker2.ts index 041a4461..284811b8 100644 --- a/ts/test/worker2.ts +++ b/ts/test/worker2.ts @@ -24,8 +24,22 @@ time.start({ useCPED: useCPED, }); -parentPort?.on('message', () => { - void delay(50).then(() => { - parentPort?.postMessage('hello'); +function listen() { + parentPort?.on('message', () => { + void delay(50).then(() => { + parentPort?.postMessage('hello'); + }); }); -}); +} + +// Establish a sample context, and do it around the listener registration so +// the async context frame holding it stays reachable until we are terminated. +// That leaves a live PersistentContextPtr for ~WallProfiler to walk when it +// runs from the environment cleanup hook; with an empty list the walk is a +// no-op and the teardown path goes untested. +if (useCPED) { + time.runWithContext({worker: 'worker2'}, listen); +} else { + time.setContext({worker: 'worker2'}); + listen(); +} From ec1ac4cf0f5280009c086ae37594847a01140408 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Thu, 20 Aug 2026 16:55:52 +0200 Subject: [PATCH 6/6] v5.18.1 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index becc7333..972bf4cc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@datadog/pprof", - "version": "5.18.0", + "version": "5.18.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@datadog/pprof", - "version": "5.18.0", + "version": "5.18.1", "license": "Apache-2.0", "dependencies": { "node-gyp-build": "^4.8.4", diff --git a/package.json b/package.json index e57b68a0..85062239 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@datadog/pprof", - "version": "5.18.0", + "version": "5.18.1", "description": "pprof support for Node.js", "repository": { "type": "git",