Fixed memory leak in CombinedCancellationToken (#384)

amphp · Feb 20, 2022 · 9d5100c · 9d5100c
1 parent 591e06e
commit 9d5100c
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 5 deletions.
diff --git a/lib/CombinedCancellationToken.php b/lib/CombinedCancellationToken.php
@@ -18,15 +18,18 @@ final class CombinedCancellationToken implements CancellationToken
 
     public function __construct(CancellationToken ...$tokens)
     {
+        $thatException = &$this->exception;
+        $thatCallbacks = &$this->callbacks;
+
         foreach ($tokens as $token) {
-            $id = $token->subscribe(function (CancelledException $exception) {
-                $this->exception = $exception;
+            $id = $token->subscribe(static function (CancelledException $exception) use (&$thatException, &$thatCallbacks) {
+                $thatException = $exception;
 
-                $callbacks = $this->callbacks;
-                $this->callbacks = [];
+                $callbacks = $thatCallbacks;
+                $thatCallbacks = [];
 
                 foreach ($callbacks as $callback) {
-                    asyncCall($callback, $this->exception);
+                    asyncCall($callback, $thatException);
                 }
             });
 

diff --git a/test/CombinedCancellationTokenTest.php b/test/CombinedCancellationTokenTest.php
@@ -0,0 +1,38 @@
+<?php
+declare(strict_types=1);
+
+namespace Amp\Test;
+
+use Amp\CancellationTokenSource;
+use Amp\CombinedCancellationToken;
+
+class CombinedCancellationTokenTest extends BaseTest
+{
+    private const LOOP_COUNT = 20;
+    private const TOKENS_COUNT = 1000;
+
+    public function testBenchmark(): void
+    {
+        $firstMemoryMeasure = 0;
+
+        for ($i = 0; $i < self::LOOP_COUNT; $i++) {
+            $tokens = [];
+            for ($j = 0; $j < self::TOKENS_COUNT; $j++) {
+                $tokens[] = (new CancellationTokenSource)->getToken();
+            }
+            $combinedToken = new CombinedCancellationToken(...$tokens);
+
+            if (!$firstMemoryMeasure && $i > self::LOOP_COUNT / 2) {
+                // Warmup and store first memory usage after 50% of iterations
+                $firstMemoryMeasure = \memory_get_usage(true);
+            }
+            // Remove tokens from memory
+            unset($combinedToken);
+
+            // Asserts
+            if ($firstMemoryMeasure > 0) {
+                self::assertEquals($firstMemoryMeasure, \memory_get_usage(true));
+            }
+        }
+    }
+}