lib: clean up persisted signals when they are settled

geeksilva97 · aduh95 · commit fd55d3cbdde3 · 2024-12-10T20:30:57.000+01:00
PR-URL: #56001 Refs: #55328 Fixes: #55328 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
diff --git 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
@@ -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();
+        });
+      });
+    });
+  });
+});
