don't set a finalizer on cloned request (nodejs#4320)

tsctx · web-flow · commit c5e74641f40e · 2025-07-04T12:47:47.000-04:00
diff --git a/lib/web/fetch/body.js b/lib/web/fetch/body.js
@@ -29,17 +29,12 @@ try {
 const textEncoder = new TextEncoder()
 function noop () {}
 
-const hasFinalizationRegistry = globalThis.FinalizationRegistry
-let streamRegistry
-
-if (hasFinalizationRegistry) {
-  streamRegistry = new FinalizationRegistry((weakRef) => {
-    const stream = weakRef.deref()
-    if (stream && !stream.locked && !isDisturbed(stream) && !isErrored(stream)) {
-      stream.cancel('Response object has been garbage collected').catch(noop)
-    }
-  })
-}
+const streamRegistry = new FinalizationRegistry((weakRef) => {
+  const stream = weakRef.deref()
+  if (stream && !stream.locked && !isDisturbed(stream) && !isErrored(stream)) {
+    stream.cancel('Response object has been garbage collected').catch(noop)
+  }
+})
 
 /**
  * Extract a body with type from a byte sequence or BodyInit object
@@ -306,17 +301,13 @@ function safelyExtractBody (object, keepalive = false) {
   return extractBody(object, keepalive)
 }
 
-function cloneBody (instance, body) {
+function cloneBody (body) {
   // To clone a body body, run these steps:
 
   // https://fetch.spec.whatwg.org/#concept-body-clone
 
   // 1. Let « out1, out2 » be the result of teeing body’s stream.
-  const [out1, out2] = body.stream.tee()
-
-  if (hasFinalizationRegistry) {
-    streamRegistry.register(instance, new WeakRef(out1))
-  }
+  const { 0: out1, 1: out2 } = body.stream.tee()
 
   // 2. Set body’s stream to out1.
   body.stream = out1
@@ -548,6 +539,5 @@ module.exports = {
   cloneBody,
   mixinBody,
   streamRegistry,
-  hasFinalizationRegistry,
   bodyUnusable
 }
diff --git a/lib/web/fetch/dispatcher-weakref.js b/lib/web/fetch/dispatcher-weakref.js
diff --git a/lib/web/fetch/request.js b/lib/web/fetch/request.js
@@ -4,7 +4,6 @@
 
 const { extractBody, mixinBody, cloneBody, bodyUnusable } = require('./body')
 const { Headers, fill: fillHeaders, HeadersList, setHeadersGuard, getHeadersGuard, setHeadersList, getHeadersList } = require('./headers')
-const { FinalizationRegistry } = require('./dispatcher-weakref')()
 const util = require('../../core/util')
 const nodeUtil = require('node:util')
 const {
@@ -937,7 +936,7 @@ function cloneRequest (request) {
   // 2. If request’s body is non-null, set newRequest’s body to the
   // result of cloning request’s body.
   if (request.body != null) {
-    newRequest.body = cloneBody(newRequest, request.body)
+    newRequest.body = cloneBody(request.body)
   }
 
   // 3. Return newRequest.
diff --git a/lib/web/fetch/response.js b/lib/web/fetch/response.js
@@ -1,7 +1,7 @@
 'use strict'
 
 const { Headers, HeadersList, fill, getHeadersGuard, setHeadersGuard, setHeadersList } = require('./headers')
-const { extractBody, cloneBody, mixinBody, hasFinalizationRegistry, streamRegistry, bodyUnusable } = require('./body')
+const { extractBody, cloneBody, mixinBody, streamRegistry, bodyUnusable } = require('./body')
 const util = require('../../core/util')
 const nodeUtil = require('node:util')
 const { kEnumerableProperty } = util
@@ -352,7 +352,9 @@ function cloneResponse (response) {
   // 3. If response’s body is non-null, then set newResponse’s body to the
   // result of cloning response’s body.
   if (response.body != null) {
-    newResponse.body = cloneBody(newResponse, response.body)
+    newResponse.body = cloneBody(response.body)
+
+    streamRegistry.register(newResponse, new WeakRef(response.body.stream))
   }
 
   // 4. Return newResponse.
@@ -552,7 +554,7 @@ function fromInnerResponse (innerResponse, guard) {
   setHeadersList(headers, innerResponse.headersList)
   setHeadersGuard(headers, guard)
 
-  if (hasFinalizationRegistry && innerResponse.body?.stream) {
+  if (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
diff --git a/test/fetch/fire-and-forget.js b/test/fetch/fire-and-forget.js
@@ -3,14 +3,33 @@
 const { randomFillSync } = require('node:crypto')
 const { setTimeout: sleep } = require('node:timers/promises')
 const { test } = require('node:test')
-const { fetch, Agent, setGlobalDispatcher } = require('../..')
+const { fetch, Request, Agent, setGlobalDispatcher } = require('../..')
 const { createServer } = require('node:http')
 const { closeServerAsPromise } = require('../utils/node-http')
+const assert = require('node:assert')
 
 const blob = randomFillSync(new Uint8Array(1024 * 512))
 
 const hasGC = typeof global.gc !== 'undefined'
 
+// https://github.com/nodejs/undici/issues/4150
+test('test finalizer cloned request', async () => {
+  if (!hasGC) {
+    throw new Error('gc is not available. Run with \'--expose-gc\'.')
+  }
+
+  const request = new Request('http://localhost', { method: 'POST', body: 'Hello' })
+
+  for (let i = 0; i < 800; ++i) request.clone()
+
+  await sleep(50)
+
+  // eslint-disable-next-line no-undef
+  gc(true)
+
+  assert.strictEqual(request.bodyUsed, false)
+})
+
 test('does not need the body to be consumed to continue', { timeout: 180_000 }, async (t) => {
   if (!hasGC) {
     throw new Error('gc is not available. Run with \'--expose-gc\'.')
