From fd55d3cbdde39a8a538b8ae911377f81e328f56f Mon Sep 17 00:00:00 2001
From: "Edigleysson Silva (Edy)" <edigleyssonsilva@gmail.com>
Date: Sun, 8 Dec 2024 09:24:09 -0300
Subject: [PATCH] lib: clean up persisted signals when they are settled
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PR-URL: https://github.com/nodejs/node/pull/56001
Refs: https://github.com/nodejs/node/issues/55328
Fixes: https://github.com/nodejs/node/issues/55328
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Jason Zhang <xzha4350@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
---
 lib/internal/abort_controller.js              | 22 ++++++++
 .../test-abortsignal-drop-settled-signals.mjs | 55 +++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js
index 6dd40cb00e9950..8ec9034a4f9352 100644
--- a/lib/internal/abort_controller.js
+++ b/lib/internal/abort_controller.js
@@ -96,8 +96,21 @@ const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry((signalWeak
     }
   });
 });
+
 const gcPersistentSignals = new SafeSet();
 
+const sourceSignalsCleanupRegistry = new SafeFinalizationRegistry(({ sourceSignalRef, composedSignalRef }) => {
+  const composedSignal = composedSignalRef.deref();
+  if (composedSignal !== undefined) {
+    composedSignal[kSourceSignals].delete(sourceSignalRef);
+
+    if (composedSignal[kSourceSignals].size === 0) {
+      // This signal will no longer abort. There's no need to keep it in the gcPersistentSignals set.
+      gcPersistentSignals.delete(composedSignal);
+    }
+  }
+});
+
 const kAborted = Symbol('kAborted');
 const kReason = Symbol('kReason');
 const kCloneData = Symbol('kCloneData');
@@ -260,6 +273,10 @@ class AbortSignal extends EventTarget {
         resultSignal[kSourceSignals].add(signalWeakRef);
         signal[kDependantSignals].add(resultSignalWeakRef);
         dependantSignalsCleanupRegistry.register(resultSignal, signalWeakRef);
+        sourceSignalsCleanupRegistry.register(signal, {
+          sourceSignalRef: signalWeakRef,
+          composedSignalRef: resultSignalWeakRef,
+        });
       } else if (!signal[kSourceSignals]) {
         continue;
       } else {
@@ -277,6 +294,10 @@ class AbortSignal extends EventTarget {
           resultSignal[kSourceSignals].add(sourceSignalWeakRef);
           sourceSignal[kDependantSignals].add(resultSignalWeakRef);
           dependantSignalsCleanupRegistry.register(resultSignal, sourceSignalWeakRef);
+          sourceSignalsCleanupRegistry.register(signal, {
+            sourceSignalRef: sourceSignalWeakRef,
+            composedSignalRef: resultSignalWeakRef,
+          });
         }
       }
     }
@@ -436,6 +457,7 @@ class AbortController {
    */
   get signal() {
     this.#signal ??= new AbortSignal(kDontThrowSymbol);
+
     return this.#signal;
   }
 
diff --git a/test/parallel/test-abortsignal-drop-settled-signals.mjs b/test/parallel/test-abortsignal-drop-settled-signals.mjs
index 728002b51d30d5..f829fb0a9173fa 100644
--- a/test/parallel/test-abortsignal-drop-settled-signals.mjs
+++ b/test/parallel/test-abortsignal-drop-settled-signals.mjs
@@ -64,6 +64,41 @@ function runShortLivedSourceSignal(limit, done) {
   run(1);
 };
 
+function runWithOrphanListeners(limit, done) {
+  let composedSignalRef;
+  const composedSignalRefs = [];
+  const handler = () => { };
+
+  function run(iteration) {
+    const ac = new AbortController();
+    if (iteration > limit) {
+      setImmediate(() => {
+        global.gc();
+        setImmediate(() => {
+          global.gc();
+
+          done(composedSignalRefs);
+        });
+      });
+      return;
+    }
+
+    composedSignalRef = new WeakRef(AbortSignal.any([ac.signal]));
+    composedSignalRef.deref().addEventListener('abort', handler);
+
+    const otherComposedSignalRef = new WeakRef(AbortSignal.any([composedSignalRef.deref()]));
+    otherComposedSignalRef.deref().addEventListener('abort', handler);
+
+    composedSignalRefs.push(composedSignalRef, otherComposedSignalRef);
+
+    setImmediate(() => {
+      run(iteration + 1);
+    });
+  }
+
+  run(1);
+}
+
 const limit = 10_000;
 
 describe('when there is a long-lived signal', () => {
@@ -120,3 +155,23 @@ it('drops settled dependant signals when signal is composite', (t, done) => {
     });
   });
 });
+
+it('drops settled signals even when there are listeners', (t, done) => {
+  runWithOrphanListeners(limit, (signalRefs) => {
+    setImmediate(() => {
+      global.gc();
+      setImmediate(() => {
+        global.gc(); // One more call needed to clean up the deeper composed signals
+        setImmediate(() => {
+          global.gc(); // One more call needed to clean up the deeper composed signals
+
+          const unGCedSignals = [...signalRefs].filter((ref) => ref.deref());
+
+          t.assert.strictEqual(unGCedSignals.length, 0);
+
+          done();
+        });
+      });
+    });
+  });
+});