Merge pull request #118 from clue-labs/stack

Improve memory consumption by hiding resolver and canceller references from call stack on PHP 7+

Author: WyriHaximus

src/Promise.php

@@ -15,7 +15,13 @@ class Promise implements ExtendedPromiseInterface, CancellablePromiseInterface
     public function __construct(callable $resolver, callable $canceller = null)
     {
         $this->canceller = $canceller;
-        $this->call($resolver);
+
+        // Explicitly overwrite arguments with null values before invoking
+        // resolver function. This ensure that these arguments do not show up
+        // in the stack trace in PHP 7+ only.
+        $cb = $resolver;
+        $resolver = $canceller = null;
+        $this->call($cb);
     }
 
     public function then(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null)
@@ -228,8 +234,13 @@ private function extract($promise)
         return $promise;
     }
 
-    private function call(callable $callback)
+    private function call(callable $cb)
     {
+        // Explicitly overwrite argument with null value. This ensure that this
+        // argument does not show up in the stack trace in PHP 7+ only.
+        $callback = $cb;
+        $cb = null;
+
         // Use reflection to inspect number of arguments expected by this callback.
         // We did some careful benchmarking here: Using reflection to avoid unneeded
         // function arguments is actually faster than blindly passing them.

tests/PromiseTest.php

@@ -122,6 +122,50 @@ public function shouldRejectWithoutCreatingGarbageCyclesIfResolverThrowsExceptio
         $this->assertSame(0, gc_collect_cycles());
     }
 
+    /**
+     * Test that checks number of garbage cycles after throwing from a canceller
+     * that explicitly uses a reference to the promise. This is rather synthetic,
+     * actual use cases often have implicit (hidden) references which ought not
+     * to be stored in the stack trace.
+     *
+     * Reassigned arguments only show up in the stack trace in PHP 7, so we can't
+     * avoid this on legacy PHP. As an alternative, consider explicitly unsetting
+     * any references before throwing.
+     *
+     * @test
+     * @requires PHP 7
+     */
+    public function shouldRejectWithoutCreatingGarbageCyclesIfCancellerWithReferenceThrowsException()
+    {
+        gc_collect_cycles();
+
+        $promise = new Promise(function () {}, function () use (&$promise) {
+            throw new \Exception('foo');
+        });
+        $promise->cancel();
+        unset($promise);
+
+        $this->assertSame(0, gc_collect_cycles());
+    }
+
+    /**
+     * @test
+     * @requires PHP 7
+     * @see self::shouldRejectWithoutCreatingGarbageCyclesIfCancellerWithReferenceThrowsException
+     */
+    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 */
     public function shouldIgnoreNotifyAfterReject()
     {