Clean up canceller function references when they are no longer needed

reactphp · May 6, 2018 · fbdf0aa · fbdf0aa
1 parent e521e57
commit fbdf0aa
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 2 deletions.
diff --git a/src/Deferred.php b/src/Deferred.php
@@ -23,6 +23,7 @@ public function promise()
                 $this->rejectCallback  = $reject;
                 $this->notifyCallback  = $notify;
             }, $this->canceller);
+            $this->canceller = null;
         }
 
         return $this->promise;

diff --git a/src/Promise.php b/src/Promise.php
@@ -202,6 +202,8 @@ private function settle(ExtendedPromiseInterface $promise)
 
         if ($promise instanceof self) {
             $promise->requiredCancelRequests++;
+        } else {
+            $this->canceller = null;
         }
 
         $handlers = $this->handlers;

diff --git a/tests/DeferredTest.php b/tests/DeferredTest.php
@@ -39,4 +39,41 @@ public function progressIsAnAliasForNotify()
 
         $deferred->progress($sentinel);
     }
+
+    /** @test */
+    public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerRejectsWithException()
+    {
+        gc_collect_cycles();
+        $deferred = new Deferred(function ($resolve, $reject) {
+            $reject(new \Exception('foo'));
+        });
+        $deferred->promise()->cancel();
+        unset($deferred);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldRejectWithoutCreatingGarbageCyclesIfParentCancellerRejectsWithException()
+    {
+        gc_collect_cycles();
+        $deferred = new Deferred(function ($resolve, $reject) {
+            $reject(new \Exception('foo'));
+        });
+        $deferred->promise()->then()->cancel();
+        unset($deferred);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerHoldsReferenceAndExplicitlyRejectWithException()
+    {
+        gc_collect_cycles();
+        $deferred = new Deferred(function () use (&$deferred) { });
+        $deferred->reject(new \Exception('foo'));
+        unset($deferred);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
 }
diff --git a/tests/PromiseTest.php b/tests/PromiseTest.php
@@ -138,7 +138,6 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfResolverThrowsExceptio
     public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerWithReferenceThrowsException()
     {
         gc_collect_cycles();
-
         $promise = new Promise(function () {}, function () use (&$promise) {
             throw new \Exception('foo');
         });
@@ -156,11 +155,25 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerWithReference
     public function shouldRejectWithoutCreatingGarbageCyclesIfResolverWithReferenceThrowsException()
     {
         gc_collect_cycles();
-
         $promise = new Promise(function () use (&$promise) {
             throw new \Exception('foo');
         });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
 
+    /**
+     * @test
+     * @requires PHP 7
+     * @see self::shouldRejectWithoutCreatingGarbageCyclesIfCancellerWithReferenceThrowsException
+     */
+    public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerHoldsReferenceAndResolverThrowsException()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function () {
+            throw new \Exception('foo');
+        }, function () use (&$promise) { });
         unset($promise);
 
         $this->assertSame(0, gc_collect_cycles());