lib: handle composed signals finalization

nodejs · Nov 28, 2024 · c11785e · c11785e
1 parent 9d3074e
commit c11785e
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 6 deletions.
diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js
@@ -98,7 +98,7 @@ const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry((signalWeak
 
 const gcPersistentSignals = new SafeSet();
 
-const finalizer = new SafeFinalizationRegistry(({ sourceSignalRef, composedSignalRef }) => {
+const sourceSignalsCleanupRegistry = new SafeFinalizationRegistry(({ sourceSignalRef, composedSignalRef }) => {
   const composedSignal = composedSignalRef.deref();
   if (composedSignal !== undefined) {
     composedSignal[kSourceSignals].delete(sourceSignalRef);
@@ -271,7 +271,10 @@ class AbortSignal extends EventTarget {
         resultSignal[kSourceSignals].add(signalWeakRef);
         signal[kDependantSignals].add(resultSignalWeakRef);
         dependantSignalsCleanupRegistry.register(resultSignal, signalWeakRef);
-        finalizer.register(signal, { sourceSignalRef: signalWeakRef, composedSignalRef: resultSignalWeakRef });
+        sourceSignalsCleanupRegistry.register(signal, {
+          sourceSignalRef: signalWeakRef,
+          composedSignalRef: resultSignalWeakRef,
+        });
       } else if (!signal[kSourceSignals]) {
         continue;
       } else {
@@ -289,6 +292,10 @@ class AbortSignal extends EventTarget {
           resultSignal[kSourceSignals].add(sourceSignalWeakRef);
           sourceSignal[kDependantSignals].add(resultSignalWeakRef);
           dependantSignalsCleanupRegistry.register(resultSignal, sourceSignalWeakRef);
+          sourceSignalsCleanupRegistry.register(signal, {
+            sourceSignalRef: sourceSignalWeakRef,
+            composedSignalRef: resultSignalWeakRef,
+          });
         }
       }
     }

diff --git a/test/parallel/test-abortsignal-drop-settled-signals.mjs b/test/parallel/test-abortsignal-drop-settled-signals.mjs
@@ -86,7 +86,10 @@ function runWithOrphanListeners(limit, done) {
     composedSignalRef = new WeakRef(AbortSignal.any([ac.signal]));
     composedSignalRef.deref().addEventListener('abort', handler);
 
-    composedSignalRefs.push(composedSignalRef);
+    const otherComposedSignalRef = new WeakRef(AbortSignal.any([composedSignalRef.deref()]));
+    otherComposedSignalRef.deref().addEventListener('abort', handler);
+
+    composedSignalRefs.push(composedSignalRef, otherComposedSignalRef);
 
     setImmediate(() => {
       run(iteration + 1);
@@ -157,12 +160,18 @@ 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());
+          const unGCedSignals = [...signalRefs].filter((ref) => ref.deref());
 
-      t.assert.strictEqual(unGCedSignals.length, 0);
+          t.assert.strictEqual(unGCedSignals.length, 0);
 
-      done();
+          done();
+        });
+      });
     });
   });
 });