Merge pull request #34 from clue-labs/garbage

Improve memory consumption by cleaning up garbage references to pending promise without canceller
reactphp · Jun 13, 2018 · 170fb93 · 170fb93
2 parents c217811 + 28dc64e
commit 170fb93
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 4 deletions.
diff --git a/composer.json b/composer.json
@@ -20,7 +20,7 @@
     "require": {
         "php": ">=5.3",
         "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5",
-        "react/promise": "^2.6.0 || ^1.2.1"
+        "react/promise": "^2.7.0 || ^1.2.1"
     },
     "require-dev": {
         "phpunit/phpunit": "^6.4 || ^5.7 || ^4.8.35"

diff --git a/src/functions.php b/src/functions.php
@@ -23,7 +23,7 @@ function timeout(PromiseInterface $promise, $time, LoopInterface $loop)
 
     return new Promise(function ($resolve, $reject) use ($loop, $time, $promise) {
         $timer = null;
-        $promise->then(function ($v) use (&$timer, $loop, $resolve) {
+        $promise = $promise->then(function ($v) use (&$timer, $loop, $resolve) {
             if ($timer) {
                 $loop->cancelTimer($timer);
             }

diff --git a/tests/FunctionTimeoutTest.php b/tests/FunctionTimeoutTest.php
@@ -71,10 +71,13 @@ public function testPendingWillRejectOnTimeout()
         $this->expectPromiseRejected($promise);
     }
 
-    public function testPendingCancellableWillBeCancelledOnTimeout()
+    public function testPendingCancellableWillBeCancelledThroughFollowerOnTimeout()
     {
+        $cancellable = $this->getMockBuilder('React\Promise\CancellablePromiseInterface')->getMock();
+        $cancellable->expects($this->once())->method('cancel');
+
         $promise = $this->getMockBuilder('React\Promise\CancellablePromiseInterface')->getMock();
-        $promise->expects($this->once())->method('cancel');
+        $promise->expects($this->once())->method('then')->willReturn($cancellable);
 
         Timer\timeout($promise, 0.01, $this->loop);
 
@@ -223,6 +226,44 @@ public function testWaitingForPromiseToTimeoutDoesNotLeaveGarbageCycles()
         $this->assertEquals(0, gc_collect_cycles());
     }
 
+    public function testWaitingForPromiseToTimeoutWithoutCancellerDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = new \React\Promise\Promise(function () { });
+
+        $promise = Timer\timeout($promise, 0.01, $this->loop);
+
+        $this->loop->run();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
+    public function testWaitingForPromiseToTimeoutWithNoOpCancellerDoesNotLeaveGarbageCycles()
+    {
+        if (class_exists('React\Promise\When')) {
+            $this->markTestSkipped('Not supported on legacy Promise v1 API');
+        }
+
+        gc_collect_cycles();
+
+        $promise = new \React\Promise\Promise(function () { }, function () {
+            // no-op
+        });
+
+        $promise = Timer\timeout($promise, 0.01, $this->loop);
+
+        $this->loop->run();
+        unset($promise);
+
+        $this->assertEquals(0, gc_collect_cycles());
+    }
+
     public function testCancellingPromiseDoesNotLeaveGarbageCycles()
     {
         if (class_exists('React\Promise\When')) {