fix: unregister when signal is aborted

nodejs · Dec 21, 2023 · 1510ec3 · 1510ec3
1 parent c5c6648
commit 1510ec3
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 1 deletion.
diff --git a/lib/compat/dispatcher-weakref.js b/lib/compat/dispatcher-weakref.js
@@ -28,6 +28,8 @@ class CompatFinalizer {
       })
     }
   }
+
+  unregister (key) {}
 }
 
 module.exports = function () {

diff --git a/lib/fetch/request.js b/lib/fetch/request.js
@@ -35,6 +35,12 @@ const { getMaxListeners, setMaxListeners, getEventListeners, defaultMaxListeners
 const kAbortController = Symbol('abortController')
 
 const requestFinalizer = new FinalizationRegistry(({ signal, abort }) => {
+  // Currently FinalizationRegistry has a problem and will explicitly call unregister.
+  // https://github.com/nodejs/node/issues/49344
+  // https://github.com/nodejs/node/issues/47748
+  // It will be removed in the future.
+  // Note: The unregister key is abort.
+  requestFinalizer.unregister(abort)
   signal.removeEventListener('abort', abort)
 })
 
@@ -371,6 +377,7 @@ class Request {
         const abort = function () {
           const ac = acRef.deref()
           if (ac !== undefined) {
+            // requestFinalizer.unregister(ac)
             ac.abort(this.reason)
           }
         }
@@ -388,7 +395,8 @@ class Request {
         } catch {}
 
         util.addAbortListener(signal, abort)
-        requestFinalizer.register(ac, { signal, abort })
+        // abort is used as the unregister key.
+        requestFinalizer.register(ac, { signal, abort }, abort)
       }
     }