Repository navigation
Conversation
…l#97941) Fixes vercel#97934. The default use cache handler uses live ReadableStream objects. Those streams retain references to the request’s async context and closed HTTP response, causing memory to grow with cached entries and potentially produce OOMs. The fix converts each stream into a plain Buffer before caching it. Every cache hit then gets a fresh temporary stream created from those bytes. The content remains cached, but completed requests can now be garbage-collected. --- Basically - the LRU cache stores live ReadableStream objects, which keep completed request contexts and closed responses in memory until that cache is evicted. (cherry picked from commit c5b960e)
Contributor
Failing test suitesCommit: be183fb | About building and testing Next.js
Expand output● app dir - metadata dynamic routes with async deps › should render page with og:image meta tag when opengraph-image has async dependencies ● app dir - metadata dynamic routes with async deps › should serve the opengraph-image route as a valid image
Expand output● app dir - metadata dynamic routes with async deps › should render page with og:image meta tag when opengraph-image has async dependencies ● app dir - metadata dynamic routes with async deps › should serve the opengraph-image route as a valid image
Expand output● app dir - metadata dynamic routes with async deps › should render page with og:image meta tag when opengraph-image has async dependencies ● app dir - metadata dynamic routes with async deps › should serve the opengraph-image route as a valid image
Expand output● app dir - metadata dynamic routes › robots.txt › should handle robots.[ext] dynamic routes ● app dir - metadata dynamic routes › sitemap › should handle sitemap.[ext] dynamic routes ● app dir - metadata dynamic routes › sitemap › should support generate multi sitemaps with generateSitemaps ● app dir - metadata dynamic routes › sitemap › should 404 for non-existing id from generateImageMetadata ● app dir - metadata dynamic routes › sitemap › should not throw if client components are imported but not used in sitemap ● app dir - metadata dynamic routes › sitemap › should support alternate.languages in sitemap ● app dir - metadata dynamic routes › sitemap › should support images in sitemap ● app dir - metadata dynamic routes › sitemap › should support videos in sitemap ● app dir - metadata dynamic routes › social image routes › should handle manifest.[ext] dynamic routes ... truncated ...
Expand output● app dir - metadata dynamic routes with async deps › should render page with og:image meta tag when opengraph-image has async dependencies ● app dir - metadata dynamic routes with async deps › should serve the opengraph-image route as a valid image
Expand output● app dir - metadata dynamic routes › robots.txt › should handle robots.[ext] dynamic routes ● app dir - metadata dynamic routes › sitemap › should handle sitemap.[ext] dynamic routes ● app dir - metadata dynamic routes › sitemap › should support generate multi sitemaps with generateSitemaps ● app dir - metadata dynamic routes › sitemap › should 404 for non-existing id from generateImageMetadata ● app dir - metadata dynamic routes › sitemap › should not throw if client components are imported but not used in sitemap ● app dir - metadata dynamic routes › sitemap › should support alternate.languages in sitemap ● app dir - metadata dynamic routes › sitemap › should support images in sitemap ● app dir - metadata dynamic routes › sitemap › should support videos in sitemap ... truncated to fit in one GitHub comment ... |
|
This fixes the retention it targets, but on Node 24.18+ the way it stores the value makes the cache hold far more than
I measured it on a
The heap snapshot of the second row has 10,993 Copying the value into its own allocation before storing it is enough: const pooled = await streamToBuffer(entry.value)
const value = Buffer.allocUnsafeSlow(pooled.byteLength)
pooled.copy(value)I'd keep the copy here rather than in It reproduces without Next. 20,000 values of ~2 KB, built with Measured with next-leak, a CLI I maintain: |
|
Hi ! We're hitting this in production on 16.3.6 (Vercel, Fluid compute, cacheComponents: true). Instances grow steadily until the 2 GB limit, then 300s timeouts and "instance was killed because it ran out of available memory". Local repro on a production build (
|
This branch has not been deployed
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
4 participants
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.
Backports #97941 ("Fix request-context retention in the default use cache handler") to
next-16-3.Cherry-picked from c5b960e, no conflicts.
packages/next/src/server/lib/cache-handlers/default.tsonnext-16-3is byte-identical to its state right before #97941.streamFromBufferandstreamToBufferalready exist innode-web-streams-helper.tson 16.3, and the new test only imports Node built-ins and neighbouring modules.Why this is worth a backport
On 16.3.x, every on-demand prerender of a route that uses
'use cache'keeps its whole request alive: the closedServerResponse, the async context and the rendered HTML, until the LRU entry is evicted. A crawler walking long-tail URLs, which is normal traffic for an SEO-facing site, takes a Cloud Run instance down within minutes. #97476 (backported in #98448) doesn't cover this path.Reproduction (16.3.6)
Setup:
cacheComponents: true, and one routeapp/p/[id]/page.tsxthat awaits a'use cache'function withcacheLife('hours')and hasgenerateStaticParamsreturning[{ id: '0' }].next build && next startin a container with 1 CPU and 1 GiB.autocannon -c 8against/p/1,/p/2, … (a unique URL per request).dist)'use cache', 16.3.6A heap snapshot on unpatched 16.3.6 showed complete rendered HTML documents reachable through the request's work store:
kResourceStore→cacheController→AbortSignal.reason→ the render. That matches the description in #97934.Disclosure: this PR and its measurements were prepared with an AI coding agent (Claude Code) and reviewed by me. We run a multi-tenant Next.js 16 app on Firebase App Hosting, and this class of memory growth under crawler traffic has been hurting us for months. A 16.3.x release with this fix would let us stay on stable instead of a canary.