Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/web/cache/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -794,9 +794,9 @@ class Cache {
// 5.5.2
for (const response of responses) {
// 5.5.2.1
const responseObject = fromInnerResponse(response, 'immutable')
const responseObject = fromInnerResponse(cloneResponse(response), 'immutable')

responseList.push(responseObject.clone())
responseList.push(responseObject)

if (responseList.length >= maxResponses) {
break
Expand Down
3 changes: 2 additions & 1 deletion lib/web/fetch/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,8 @@ function fromInnerResponse (innerResponse, guard) {
setHeadersList(headers, innerResponse.headersList)
setHeadersGuard(headers, guard)

if (innerResponse.body?.stream) {
// Note: If innerResponse's urlList contains a URL, it is a fetch response.
if (innerResponse.urlList.length !== 0 && innerResponse.body?.stream) {
// If the target (response) is reclaimed, the cleanup callback may be called at some point with
// the held value provided for it (innerResponse.body.stream). The held value can be any value:
// a primitive or an object, even undefined. If the held value is an object, the registry keeps
Expand Down
42 changes: 42 additions & 0 deletions test/cache/cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,52 @@
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), {
name: 'TypeError',
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`)
}
})
Loading