Fix request context retention in cookies/headers/draftMode - #96800
Draft
marcoshernanz wants to merge 1 commit into
Draft
Fix request context retention in cookies/headers/draftMode#96800marcoshernanz wants to merge 1 commit into
marcoshernanz wants to merge 1 commit into
Conversation
Contributor
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
Build Cache
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📝 Changed Files (2 files)Files with changes:
View diffspages-api-tu..ntime.dev.jsDiff too large to display pages-turbo...ntime.dev.jsDiff too large to display 📎 Tarball URLCommit: adb41b6 |
Contributor
Tests PassedCommit: adb41b6 |
marcoshernanz
force-pushed
the
marcos/memleak-request-apis
branch
from
August 6, 2026 08:40
bbc7023 to
f790b90
Compare
Immediately resolved cookies(), headers(), and draftMode() promises were created while the work and work-unit async-local stores were active. For cookies() and headers() the promise is cached in the module-scope CachedCookies/CachedHeaders WeakMaps, so it retained the originating request context through its async-hooks resource store for as long as the WeakMap entry lived - the same leak class as #96533 (fixed for params/searchParams in #96573), in the request APIs that PR did not cover. Per-request keys make the retention transient, but the WeakMap ephemeron cycle (the promise resolves to its key object and captures the store that owns the key) defeats minor GC, so every request that reads them leaves its full request context in old space until a major GC - the same accumulation mechanism measured in #96533 on Node 22. Create these promises inside the work store's clean async snapshot instead, matching the approach of #96573, so framework-created request API promises never root the request context that produced them. No WeakRef accessors are needed: unlike the searchParams erroring proxy, these production paths create plain resolved promises with no deferred store access. draftMode() gets the same treatment for consistency, and its vestigial draft-mode cache is removed: CachedDraftModes has been get-only since the .set was dropped in #84179, so no immortal-key retention ever existed there; the clean snapshot keeps all four APIs uniform regardless. Adds an async-hooks e2e regression test proving ordinary render promises capture the stores while cookies()/headers()/draftMode() promises do not, covering a dynamic request (request store, exercises the per-request WeakMap keys) and a force-static page prerendered at build time (exercises the forceStatic override paths). Validation: the regression test fails without the fix (both cases) and passes with it in both bundlers; existing suites for the touched APIs pass (draft-mode, draft-mode-middleware, dynamic-data, headers-static-bailout); tsc/Prettier/ESLint clean; autoreview panel findings addressed. Related: #96533, #96573 Co-Authored-By: Marcos Hernanz <96699542+marcoshernanz@users.noreply.github.com>
marcoshernanz
force-pushed
the
marcos/memleak-request-apis
branch
from
August 6, 2026 09:06
f790b90 to
adb41b6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Prevent immediately resolved
cookies(),headers(), anddraftMode()promises from retaining the request async context.This is the same leak class as #96533 (fixed for
params/searchParamsin #96573), in the request APIs that PR did not cover.Why?
cookies()andheaders()cache their promises in the module-scopeCachedCookies/CachedHeadersWeakMaps. Each cached promise was created while the work and work-unit async-local stores were active, so the promise's async-hooks resource store keeps the originating request context — and everything it references — alive for as long as the WeakMap entry lives. Per-request keys make the retention transient, but the WeakMap ephemeron cycle (the promise resolves to its key object and captures the store that owns the key) defeats minor GC, so every request that reads them leaves its full request context in old space until a major GC — the same accumulation mechanism measured in #96533 on Node 22.draftMode()gets the same treatment for consistency: itsCachedDraftModesWeakMap turned out to be get-only vestigial code (the.setwas dropped in #84179), so no immortal-key retention ever existed there — the vestigial map is removed, and the clean snapshot keeps all four request APIs uniform.How?
workStore.runInCleanSnapshot), so they capture no async-local stores — matching the approach of Fix request context retention in request props #96573.WeakRefaccessors are needed: unlike thesearchParamserroring proxy, these production paths create plain resolved promises with no deferred store access.CachedDraftModes/NullDraftModecache (audit finding: never written since [Breaking] Remove deprecated sync access to Dynamic APIs #84179).cookies()/headers()/draftMode()promises do not, covering:force-staticpage prerendered at build time (exercises theforceStaticoverride paths).Scope boundary (reviewer-noted follow-up)
Staged-rendering paths (
stagedRendering.delayUntilStage) andrequestStore.asyncApiPromisesalso create promises in the active async context. Those live on the request store itself, so their retention is bounded by the owning request's lifetime — the same boundary #96573 chose forparams/searchParams. If the staged paths should be cleaned too, that change belongs across all request APIs (including #96573's) as its own follow-up.Validation
true) and passes with it, in both bundlers:pnpm test-start test/e2e/app-dir/request-api-context-retention/request-api-context-retention.test.tspnpm test-start-turbo test/e2e/app-dir/request-api-context-retention/request-api-context-retention.test.tsdraft-mode,draft-mode-middleware,dynamic-data,headers-static-bailout(33+ tests).tsc --noEmit -p packages/next/tsconfig.jsonclean; Prettier and ESLint clean; autoreview panel findings addressed (vestigial draft-mode cache removed, test comment corrected).Related: #96533, #96573