From fcf306b976446176db39ca13db6522f66a895876 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 29 Dec 2025 05:07:24 +0100 Subject: [PATCH] fix(cache): regenerate stream from source when cache.match is called after GC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cached response's stream could be cancelled by the FinalizationRegistry when a temporary Response object wrapping the inner response was garbage collected. This caused cache.match() to throw "Body has already been consumed" after garbage collection ran between calls. The fix checks if the cached stream is unusable (locked or disturbed) but the body source is available (which cache.put() always stores), and if so, recreates the stream from the source before creating the Response object. Fixes: https://github.com/nodejs/undici/issues/4710 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 Signed-off-by: Matteo Collina --- lib/web/cache/cache.js | 32 ++++++++++++++++++++++++++++++-- test/cache/cache.js | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/lib/web/cache/cache.js b/lib/web/cache/cache.js index 70a3787a71d..d1e1e3c5dee 100644 --- a/lib/web/cache/cache.js +++ b/lib/web/cache/cache.js @@ -4,12 +4,12 @@ const assert = require('node:assert') const { kConstruct } = require('../../core/symbols') const { urlEquals, getFieldValues } = require('./util') -const { kEnumerableProperty, isDisturbed } = require('../../core/util') +const { kEnumerableProperty, isDisturbed, isBuffer } = require('../../core/util') const { webidl } = require('../webidl') const { cloneResponse, fromInnerResponse, getResponseState } = require('../fetch/response') const { Request, fromInnerRequest, getRequestState } = require('../fetch/request') const { fetching } = require('../fetch/index') -const { urlIsHttpHttpsScheme, readAllBytes } = require('../fetch/util') +const { urlIsHttpHttpsScheme, readAllBytes, readableStreamClose } = require('../fetch/util') const { createDeferredPromise } = require('../../util/promise') /** @@ -793,6 +793,34 @@ class Cache { // 5.5.2 for (const response of responses) { + // Fix for https://github.com/nodejs/undici/issues/4710 + // The cached response's stream may have been cancelled by the FinalizationRegistry + // when a previous Response object wrapping this inner response was garbage collected. + // If the stream is unusable but we have the body source (which cache.put() always + // stores), recreate the stream from the source. + if (response.body != null) { + const { stream, source } = response.body + const streamUnusable = stream.locked || isDisturbed(stream) + + if (streamUnusable && (typeof source === 'string' || isBuffer(source))) { + response.body.stream = new ReadableStream({ + pull (controller) { + const buffer = typeof source === 'string' + ? Buffer.from(source, 'utf-8') + : source + + if (buffer.byteLength) { + controller.enqueue(new Uint8Array(buffer)) + } + + queueMicrotask(() => readableStreamClose(controller)) + }, + start () {}, + type: 'bytes' + }) + } + } + // 5.5.2.1 const responseObject = fromInnerResponse(response, 'immutable') diff --git a/test/cache/cache.js b/test/cache/cache.js index 4a6b1ba960f..1a0be15cc68 100644 --- a/test/cache/cache.js +++ b/test/cache/cache.js @@ -3,6 +3,7 @@ const { throws } = require('node:assert') const { test } = require('node:test') const { Cache } = require('../../lib/web/cache/cache') +const { caches, Response } = require('../../') test('constructor', () => { throws(() => new Cache(null), { @@ -10,3 +11,44 @@ test('constructor', () => { message: 'TypeError: Illegal constructor' }) }) + +// https://github.com/nodejs/undici/issues/4710 +test('cache.match should work after garbage collection', async (t) => { + const cache = await caches.open('test-gc-cache') + + t.after(async () => { + await caches.delete('test-gc-cache') + }) + + const url = 'https://example.com/test-gc' + const testData = { answer: 42 } + + await cache.put(url, Response.json(testData)) + + // Call match multiple times with GC pressure between calls + // The bug manifests when the temporary Response object from fromInnerResponse() + // is garbage collected, which triggers the FinalizationRegistry to cancel + // the cached stream. + for (let i = 0; i < 20; i++) { + // Create significant memory pressure to trigger GC + // eslint-disable-next-line no-unused-vars + const garbage = Array.from({ length: 30000 }, () => ({ value: Math.random() })) + + // Force GC if available (run with --expose-gc) + if (global.gc) { + global.gc() + } + + // Delay to allow FinalizationRegistry callbacks to run + // The bug requires time for the GC to collect the temporary Response + // and for the finalization callback to cancel the stream + await new Promise((resolve) => setTimeout(resolve, 10)) + + // This should not throw "Body has already been consumed" + const match = await cache.match(url) + t.assert.ok(match, `Iteration ${i}: match should return a response`) + + const result = await match.json() + t.assert.deepStrictEqual(result, testData, `Iteration ${i}: response body should match`) + } +})