fix(fetch): hold the request's signal weakly in its finalizer

[xabierlameiro]

This relates to...

vercel/next.js#84884 — a memory leak reported eleven months ago against Next.js middleware that turns out to be this.

Related: #4984 replaces this code with AbortSignal.any([signal]) and fixes the same cycle too (I checked its branch against the repro below), but test/tls-cert-leak.js fails on that branch (3 of 3 runs here) and passes on main and with this change. This one keeps the current implementation and only changes what the held value references. It doesn't touch nodejs/node#55428, the case #4984 was opened for: dropping the Request and keeping only request.signal behaves the same before and after.

Rationale

When a Request follows a caller's signal, requestFinalizer registers the Request's own AbortController with { signal, abort } as the held value. A held value is a strong reference for as long as the target lives. So if anything reachable from the caller's signal can reach the Request — a listener that closes over it, or a property like signal.unsubscribe — the held value reaches the registry's own target, the target is never finalized, and the Request, the signal and everything they hold stay in memory for the life of the process.

It isn't hypothetical. axios's fetch adapter hands its composed signal to new Request(url, { signal }), and once a bundler inlines composeSignals into the adapter, the closure it stores on signal.unsubscribe gets the adapter's scope as its parent, which holds request. That's the whole of vercel/next.js#84884: +17 MB per 1,000 requests on Node 24.18, flat with the bundler's module merging turned off.

Plain Node, no bundler (Node 24.21, undici main):

const { Request, fetch } = require('undici')

async function once (url) {
  const controller = new AbortController()
  const request = new Request(url, { signal: controller.signal })
  controller.signal.cleanup = () => request // anything that can reach the Request
  await (await fetch(request)).text()
}

20,000 calls against a local server, heap after a full GC: 58 MB on main against 11 MB without the cleanup line. With this change it's 11 MB either way. The cleanup added in #5318 doesn't cover it, because fetch() always wraps its input in a new Request and removes that one's listener, not the listener of the Request the caller constructed.

Holding the signal weakly is safe because a signal that can still fire is reachable from whatever fires it. A controller holds its signal. AbortSignal.timeout() and AbortSignal.any() are the cases where the source doesn't, and Node keeps those alive for as long as they have an abort listener and haven't fired. I checked all three: a timeout signal, a composite and a plain controller's signal still abort a live Request after forced GCs, on main and with this change. The timeout case is in the new test, since it's the one that depends on Node keeping the signal alive.

End to end, on the reproduction from that Next.js issue: the same Next 16.3.1 build, untouched, with only the global fetch and Request replaced by undici's through --require. Retained MB per 1,000 requests:

undici	Node 24.18	Node 24.21
8.10.2	+17.69	+5.01
8.10.2 with this change	+0.05	+0.02

Changes

Features

N/A

Bug Fixes

• requestFinalizer's held value keeps the caller's signal through a WeakRef. A signal that has already been collected has no listener left to remove, so the callback does nothing in that case.
• test/fetch/request-signal-cycle.js: two cases that fail on main (a listener and a property on the signal that reference the Request), one that checks the listener is still removed from a signal that outlives the Request, and one that checks a timeout signal nobody else references still aborts the Request.

Tested on Node 22.11, 22.17, 22.23, 24.4, 24.15, 24.18 and 24.21 (not 25 or 26), and the new file 20 times in a row on 22.23 and 24.21 with no failures. npm run test:fetch passes.

Node 24 ships the v7 line (undici 7.29.1 in Node 24.21), which has the same held value. The diff doesn't apply to v7.x as is, because v7.x doesn't have #5318, but the same change made by hand passes the new tests there. I can open the backport if you want it.

Breaking Changes and Deprecations

N/A

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• [S] Benchmarked (optional)
• [S] Documented
• Review ready
• In review
• Merge ready