v5.18.1

.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<previous>-proposal
+```
+
 All subsequent steps run in the worktree directory.
 
 ### 4. Cherry-pick commits
@@ -66,7 +107,30 @@ git cherry-pick <hash1> <hash2> ...
 
 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`:
 

bindings/binding.cc

@@ -29,6 +29,40 @@
 #include <unistd.h>
 #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<v8::Value> found;
+        if (cped.As<v8::Map>()->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);
 }

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<Isolate*>(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_;
@@ -756,7 +761,7 @@ void StoreAls(const FunctionCallbackInfo<Value>& 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;

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;
   }

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",
@@ -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",

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
     '