Fix CompositeCancellation when two are cancelled

Also fixed releasing the reference to contained Cancellations as soon as one is cancelled.

src/CompositeCancellation.php

@@ -23,21 +23,34 @@ public function __construct(Cancellation ...$cancellations)
     {
         $thatException = &$this->exception;
         $thatCallbacks = &$this->callbacks;
+        $thatCancellations = &$this->cancellations;
+        $onCancel = static function (CancelledException $exception) use (
+            &$thatException,
+            &$thatCallbacks,
+            &$thatCancellations,
+        ): void {
+            if ($thatException) {
+                return;
+            }
 
-        foreach ($cancellations as $cancellation) {
-            $id = $cancellation->subscribe(static function (CancelledException $exception) use (
-                &$thatException,
-                &$thatCallbacks
-            ): void {
-                $thatException = $exception;
+            $thatException = $exception;
+
+            foreach ($thatCancellations as [$cancellation, $id]) {
+                /** @var Cancellation $cancellation */
+                $cancellation->unsubscribe($id);
+            }
 
-                foreach ($thatCallbacks as $callback) {
-                    EventLoop::queue($callback, $exception);
-                }
+            $thatCancellations = [];
 
-                $thatCallbacks = [];
-            });
+            foreach ($thatCallbacks as $callback) {
+                EventLoop::queue($callback, $exception);
+            }
+
+            $thatCallbacks = [];
+        };
 
+        foreach ($cancellations as $cancellation) {
+            $id = $cancellation->subscribe($onCancel);
             $this->cancellations[] = [$cancellation, $id];
         }
     }
@@ -48,6 +61,10 @@ public function __destruct()
             /** @var Cancellation $cancellation */
             $cancellation->unsubscribe($id);
         }
+
+        // The reference created in the constructor causes this property to persist beyond the life of this object,
+        // so explicitly removing references will speed up garbage collection.
+        $this->cancellations = [];
     }
 
     public function subscribe(\Closure $callback): string

test/Cancellation/CompositeCancellationTest.php

@@ -1,10 +1,12 @@
 <?php
 
-namespace Cancellation;
+namespace Amp\Cancellation;
 
+use Amp\CancelledException;
 use Amp\CompositeCancellation;
 use Amp\DeferredCancellation;
 use Amp\PHPUnit\AsyncTestCase;
+use Amp\PHPUnit\TestException;
 use function Amp\delay;
 
 class CompositeCancellationTest extends AsyncTestCase
@@ -38,4 +40,29 @@ public function testBenchmark(): void
             }
         }
     }
+
+    public function testCombinedWithDoubleCancellation(): void
+    {
+        $deferredCancellation1 = new DeferredCancellation();
+        $deferredCancellation2 = new DeferredCancellation();
+
+        $compositeCancellation = new CompositeCancellation(
+            $deferredCancellation1->getCancellation(),
+            $deferredCancellation2->getCancellation(),
+        );
+
+        $compositeCancellation->subscribe(
+            $this->createCallback(1, expectArgs: [self::isInstanceOf(CancelledException::class)])
+        );
+
+        $deferredCancellation1->cancel($exception = new TestException());
+        $deferredCancellation2->cancel(new TestException());
+
+        delay(0.1); // Ensure cancellation callbacks are invoked.
+
+        // Add another cancellation subscriber to ensure exception did not change.
+        $compositeCancellation->subscribe(function (CancelledException $cancelled) use ($exception) {
+            self::assertSame($exception, $cancelled->getPrevious());
+        });
+    }
 }