Skip to content

Commit cde714f

Browse files
KhafraDevmcollina
authored andcommitted
use FinalizationRegistry for cloned response body (#3458)
Signed-off-by: Matteo Collina <[email protected]>
1 parent dbb6b40 commit cde714f

File tree

4 files changed

+27
-22
lines changed

4 files changed

+27
-22
lines changed

lib/web/fetch/body.js

+21-2
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,25 @@ const { kState } = require('./symbols')
1616
const { webidl } = require('./webidl')
1717
const { Blob } = require('node:buffer')
1818
const assert = require('node:assert')
19-
const { isErrored } = require('../../core/util')
19+
const { isErrored, isDisturbed } = require('node:stream')
2020
const { isArrayBuffer } = require('node:util/types')
2121
const { serializeAMimeType } = require('./data-url')
2222
const { multipartFormDataParser } = require('./formdata-parser')
2323

2424
const textEncoder = new TextEncoder()
25+
function noop () {}
26+
27+
const hasFinalizationRegistry = globalThis.FinalizationRegistry && process.version.indexOf('v18') !== 0
28+
let streamRegistry
29+
30+
if (hasFinalizationRegistry) {
31+
streamRegistry = new FinalizationRegistry((weakRef) => {
32+
const stream = weakRef.deref()
33+
if (stream && !stream.locked && !isDisturbed(stream) && !isErrored(stream)) {
34+
stream.cancel('Response object has been garbage collected').catch(noop)
35+
}
36+
})
37+
}
2538

2639
// https://fetch.spec.whatwg.org/#concept-bodyinit-extract
2740
function extractBody (object, keepalive = false) {
@@ -264,14 +277,18 @@ function safelyExtractBody (object, keepalive = false) {
264277
return extractBody(object, keepalive)
265278
}
266279

267-
function cloneBody (body) {
280+
function cloneBody (instance, body) {
268281
// To clone a body body, run these steps:
269282

270283
// https://fetch.spec.whatwg.org/#concept-body-clone
271284

272285
// 1. Let « out1, out2 » be the result of teeing body’s stream.
273286
const [out1, out2] = body.stream.tee()
274287

288+
if (hasFinalizationRegistry) {
289+
streamRegistry.register(instance, new WeakRef(out1))
290+
}
291+
275292
// 2. Set body’s stream to out1.
276293
body.stream = out1
277294

@@ -499,5 +516,7 @@ module.exports = {
499516
safelyExtractBody,
500517
cloneBody,
501518
mixinBody,
519+
streamRegistry,
520+
hasFinalizationRegistry,
502521
bodyUnusable
503522
}

lib/web/fetch/request.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ function cloneRequest (request) {
877877
// 2. If request’s body is non-null, set newRequest’s body to the
878878
// result of cloning request’s body.
879879
if (request.body != null) {
880-
newRequest.body = cloneBody(request.body)
880+
newRequest.body = cloneBody(newRequest, request.body)
881881
}
882882

883883
// 3. Return newRequest.

lib/web/fetch/response.js

+3-18
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict'
22

33
const { Headers, HeadersList, fill, getHeadersGuard, setHeadersGuard, setHeadersList } = require('./headers')
4-
const { extractBody, cloneBody, mixinBody, bodyUnusable } = require('./body')
4+
const { extractBody, cloneBody, mixinBody, hasFinalizationRegistry, streamRegistry, bodyUnusable } = require('./body')
55
const util = require('../../core/util')
66
const nodeUtil = require('node:util')
77
const { kEnumerableProperty } = util
@@ -26,24 +26,9 @@ const { URLSerializer } = require('./data-url')
2626
const { kConstruct } = require('../../core/symbols')
2727
const assert = require('node:assert')
2828
const { types } = require('node:util')
29-
const { isDisturbed, isErrored } = require('node:stream')
3029

3130
const textEncoder = new TextEncoder('utf-8')
3231

33-
const hasFinalizationRegistry = globalThis.FinalizationRegistry && process.version.indexOf('v18') !== 0
34-
let registry
35-
36-
if (hasFinalizationRegistry) {
37-
registry = new FinalizationRegistry((weakRef) => {
38-
const stream = weakRef.deref()
39-
if (stream && !stream.locked && !isDisturbed(stream) && !isErrored(stream)) {
40-
stream.cancel('Response object has been garbage collected').catch(noop)
41-
}
42-
})
43-
}
44-
45-
function noop () {}
46-
4732
// https://fetch.spec.whatwg.org/#response-class
4833
class Response {
4934
// Creates network error Response.
@@ -327,7 +312,7 @@ function cloneResponse (response) {
327312
// 3. If response’s body is non-null, then set newResponse’s body to the
328313
// result of cloning response’s body.
329314
if (response.body != null) {
330-
newResponse.body = cloneBody(response.body)
315+
newResponse.body = cloneBody(newResponse, response.body)
331316
}
332317

333318
// 4. Return newResponse.
@@ -532,7 +517,7 @@ function fromInnerResponse (innerResponse, guard) {
532517
// a primitive or an object, even undefined. If the held value is an object, the registry keeps
533518
// a strong reference to it (so it can pass it to the cleanup callback later). Reworded from
534519
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry
535-
registry.register(response, new WeakRef(innerResponse.body.stream))
520+
streamRegistry.register(response, new WeakRef(innerResponse.body.stream))
536521
}
537522

538523
return response

test/fetch/fire-and-forget.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,9 @@ test('does not need the body to be consumed to continue', { timeout: 180_000, sk
3737
// eslint-disable-next-line no-undef
3838
gc(true)
3939
const array = new Array(batch)
40-
for (let i = 0; i < batch; i++) {
40+
for (let i = 0; i < batch; i += 2) {
4141
array[i] = fetch(url).catch(() => {})
42+
array[i + 1] = fetch(url).then(r => r.clone()).catch(() => {})
4243
}
4344
await Promise.all(array)
4445
await sleep(delay)

0 commit comments

Comments
 (0)