Merge 3dcdf8e into 15036d2

src/Promise.php

@@ -182,9 +182,9 @@ private function call(callable $callback): void
         // function arguments is actually faster than blindly passing them.
         // Also, this helps avoiding unnecessary function arguments in the call stack
         // if the callback creates an Exception (creating garbage cycles).
-        if (\is_array($callback)) {
+        if (is_array($callback)) {
             $ref = new \ReflectionMethod($callback[0], $callback[1]);
-        } elseif (\is_object($callback) && !$callback instanceof \Closure) {
+        } elseif (is_object($callback) && !$callback instanceof \Closure) {
             $ref = new \ReflectionMethod($callback, '__invoke');
         } else {
             $ref = new \ReflectionFunction($callback);
@@ -195,17 +195,68 @@ private function call(callable $callback): void
             if ($args === 0) {
                 $callback();
             } else {
+                // keep a reference to this promise instance for the static resolve/reject functions.
+                // see also resolveFunction() and rejectFunction() for more details.
+                $target =& $this;
+
                 $callback(
-                    function ($value = null) {
-                        $this->resolve($value);
-                    },
-                    function (\Throwable $reason) {
-                        $this->reject($reason);
-                    }
+                    self::resolveFunction($target),
+                    self::rejectFunction($target)
                 );
             }
         } catch (\Throwable $e) {
+            $target = null;
             $this->reject($e);
         }
     }
+
+    /**
+     * Creates a static resolver callback that is not bound to a promise instance.
+     *
+     * Moving the closure creation to a static method allows us to create a
+     * callback that is not bound to a promise instance. By passing the target
+     * promise instance by reference, we can still execute its resolving logic
+     * and still clear this reference when settling the promise. This helps
+     * avoiding garbage cycles if any callback creates an Exception.
+     *
+     * These assumptions are covered by the test suite, so if you ever feel like
+     * refactoring this, go ahead, any alternative suggestions are welcome!
+     *
+     * @param Promise $target
+     * @return callable
+     */
+    private static function resolveFunction(self &$target)
+    {
+        return function ($value = null) use (&$target) {
+            if ($target !== null) {
+                $target->settle(resolve($value));
+                $target = null;
+            }
+        };
+    }
+
+    /**
+     * Creates a static rejection callback that is not bound to a promise instance.
+     *
+     * Moving the closure creation to a static method allows us to create a
+     * callback that is not bound to a promise instance. By passing the target
+     * promise instance by reference, we can still execute its rejection logic
+     * and still clear this reference when settling the promise. This helps
+     * avoiding garbage cycles if any callback creates an Exception.
+     *
+     * These assumptions are covered by the test suite, so if you ever feel like
+     * refactoring this, go ahead, any alternative suggestions are welcome!
+     *
+     * @param Promise $target
+     * @return callable
+     */
+    private static function rejectFunction(self &$target)
+    {
+        return function ($reason = null) use (&$target) {
+            if ($target !== null) {
+                $target->reject($reason);
+                $target = null;
+            }
+        };
+    }
 }

tests/PromiseTest.php

@@ -47,8 +47,81 @@ public function shouldRejectIfResolverThrowsException()
             ->then($this->expectCallableNever(), $mock);
     }
 
+    /** @test */
+    public function shouldResolveWithoutCreatingGarbageCyclesIfResolverResolvesWithException()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function ($resolve) {
+            $resolve(new \Exception('foo'));
+        });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldRejectWithoutCreatingGarbageCyclesIfResolverThrowsExceptionWithoutResolver()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function () {
+            throw new \Exception('foo');
+        });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldRejectWithoutCreatingGarbageCyclesIfResolverRejectsWithException()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function ($resolve, $reject) {
+            $reject(new \Exception('foo'));
+        });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerRejectsWithException()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function ($resolve, $reject) { }, function ($resolve, $reject) {
+            $reject(new \Exception('foo'));
+        });
+        $promise->cancel();
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
     /** @test */
     public function shouldRejectWithoutCreatingGarbageCyclesIfResolverThrowsException()
+    {
+        gc_collect_cycles();
+        $promise = new Promise(function ($resolve, $reject) {
+            throw new \Exception('foo');
+        });
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /** @test */
+    public function shouldIgnoreNotifyAfterReject()
+    {
+        $promise = new Promise(function () { }, function ($resolve, $reject, $notify) {
+            $reject(new \Exception('foo'));
+            $notify(42);
+        });
+
+        $promise->then(null, null, $this->expectCallableNever());
+        $promise->cancel();
+    }
+
+    /** @test */
+    public function shouldFulfillIfFullfilledWithSimplePromise()
     {
         gc_collect_cycles();
         $promise = new Promise(function () {