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`) + } +})