Merge pull request #38 from jeskew/joshdifabio-fix/coroutine-leak

Fix coroutine memory leak

src/Coroutine.php

@@ -0,0 +1,151 @@
+<?php
+namespace GuzzleHttp\Promise;
+
+use Exception;
+use Generator;
+use Throwable;
+
+/**
+ * Creates a promise that is resolved using a generator that yields values or
+ * promises (somewhat similar to C#'s async keyword).
+ *
+ * When called, the coroutine function will start an instance of the generator
+ * and returns a promise that is fulfilled with its final yielded value.
+ *
+ * Control is returned back to the generator when the yielded promise settles.
+ * This can lead to less verbose code when doing lots of sequential async calls
+ * with minimal processing in between.
+ *
+ *     use GuzzleHttp\Promise;
+ *
+ *     function createPromise($value) {
+ *         return new Promise\FulfilledPromise($value);
+ *     }
+ *
+ *     $promise = Promise\coroutine(function () {
+ *         $value = (yield createPromise('a'));
+ *         try {
+ *             $value = (yield createPromise($value . 'b'));
+ *         } catch (\Exception $e) {
+ *             // The promise was rejected.
+ *         }
+ *         yield $value . 'c';
+ *     });
+ *
+ *     // Outputs "abc"
+ *     $promise->then(function ($v) { echo $v; });
+ *
+ * @param callable $generatorFn Generator function to wrap into a promise.
+ *
+ * @return Promise
+ * @link https://github.com/petkaantonov/bluebird/blob/master/API.md#generators inspiration
+ */
+final class Coroutine implements PromiseInterface
+{
+    /**
+     * @var PromiseInterface|null
+     */
+    private $currentPromise;
+
+    /**
+     * @var Generator
+     */
+    private $generator;
+
+    /**
+     * @var Promise
+     */
+    private $result;
+
+    public function __construct(callable $generatorFn)
+    {
+        $this->generator = $generatorFn();
+        $this->result = new Promise(function () {
+            while (isset($this->currentPromise)) {
+                $this->currentPromise->wait();
+            }
+        });
+        $this->nextCoroutine($this->generator->current());
+    }
+
+    public function then(
+        callable $onFulfilled = null,
+        callable $onRejected = null
+    ) {
+        return $this->result->then($onFulfilled, $onRejected);
+    }
+
+    public function otherwise(callable $onRejected)
+    {
+        return $this->result->otherwise($onRejected);
+    }
+
+    public function wait($unwrap = true)
+    {
+        return $this->result->wait($unwrap);
+    }
+
+    public function getState()
+    {
+        return $this->result->getState();
+    }
+
+    public function resolve($value)
+    {
+        $this->result->resolve($value);
+    }
+
+    public function reject($reason)
+    {
+        $this->result->reject($reason);
+    }
+
+    public function cancel()
+    {
+        $this->currentPromise->cancel();
+        $this->result->cancel();
+    }
+
+    private function nextCoroutine($yielded)
+    {
+        $this->currentPromise = promise_for($yielded)
+            ->then([$this, '_handleSuccess'], [$this, '_handleFailure']);
+    }
+
+    /**
+     * @internal
+     */
+    public function _handleSuccess($value)
+    {
+        unset($this->currentPromise);
+        try {
+            $next = $this->generator->send($value);
+            if ($this->generator->valid()) {
+                $this->nextCoroutine($next);
+            } else {
+                $this->result->resolve($value);
+            }
+        } catch (Exception $exception) {
+            $this->result->reject($exception);
+        } catch (Throwable $throwable) {
+            $this->result->reject($throwable);
+        }
+    }
+
+    /**
+     * @internal
+     */
+    public function _handleFailure($reason)
+    {
+        unset($this->currentPromise);
+        try {
+            $nextYield = $this->generator->throw(exception_for($reason));
+            // The throw was caught, so keep iterating on the coroutine
+            $this->nextCoroutine($nextYield);
+        } catch (Exception $exception) {
+            $this->result->reject($exception);
+        } catch (Throwable $throwable) {
+            $this->result->reject($throwable);
+        }
+    }
+}

src/functions.php

@@ -445,60 +445,13 @@ function is_settled(PromiseInterface $promise)
 }
 
 /**
- * Creates a promise that is resolved using a generator that yields values or
- * promises (somewhat similar to C#'s async keyword).
+ * @see Coroutine
  *
- * When called, the coroutine function will start an instance of the generator
- * and returns a promise that is fulfilled with its final yielded value.
- *
- * Control is returned back to the generator when the yielded promise settles.
- * This can lead to less verbose code when doing lots of sequential async calls
- * with minimal processing in between.
- *
- *     use GuzzleHttp\Promise;
- *
- *     function createPromise($value) {
- *         return new Promise\FulfilledPromise($value);
- *     }
- *
- *     $promise = Promise\coroutine(function () {
- *         $value = (yield createPromise('a'));
- *         try {
- *             $value = (yield createPromise($value . 'b'));
- *         } catch (\Exception $e) {
- *             // The promise was rejected.
- *         }
- *         yield $value . 'c';
- *     });
- *
- *     // Outputs "abc"
- *     $promise->then(function ($v) { echo $v; });
- *
- * @param callable $generatorFn Generator function to wrap into a promise.
+ * @param callable $generatorFn
  *
  * @return PromiseInterface
- * @link https://github.com/petkaantonov/bluebird/blob/master/API.md#generators inspiration
  */
 function coroutine(callable $generatorFn)
 {
-    $generator = $generatorFn();
-    return __next_coroutine($generator->current(), $generator)->then();
-}
-
-/** @internal */
-function __next_coroutine($yielded, \Generator $generator)
-{
-    return promise_for($yielded)->then(
-        function ($value) use ($generator) {
-            $nextYield = $generator->send($value);
-            return $generator->valid()
-                ? __next_coroutine($nextYield, $generator)
-                : $value;
-        },
-        function ($reason) use ($generator) {
-            $nextYield = $generator->throw(exception_for($reason));
-            // The throw was caught, so keep iterating on the coroutine
-            return __next_coroutine($nextYield, $generator);
-        }
-    );
+    return new Coroutine($generatorFn);
 }

tests/CoroutineTest.php

@@ -0,0 +1,65 @@
+<?php
+namespace GuzzleHttp\Promise\Tests;
+
+use GuzzleHttp\Promise\Coroutine;
+use GuzzleHttp\Promise\PromiseInterface;
+use PHPUnit_Framework_TestCase;
+use ReflectionClass;
+
+class CoroutineTest extends PHPUnit_Framework_TestCase
+{
+    /**
+     * @dataProvider promiseInterfaceMethodProvider
+     *
+     * @param string $method
+     * @param array $args
+     */
+    public function testShouldProxyPromiseMethodsToResultPromise($method, $args = [])
+    {
+        $coroutine = new Coroutine(function () { yield 0; });
+        $mockPromise = $this->getMockForAbstractClass(PromiseInterface::class);
+        call_user_func_array([$mockPromise->expects($this->once())->method($method), 'with'], $args);
+
+        $resultPromiseProp = (new ReflectionClass(Coroutine::class))->getProperty('result');
+        $resultPromiseProp->setAccessible(true);
+        $resultPromiseProp->setValue($coroutine, $mockPromise);
+
+        call_user_func_array([$coroutine, $method], $args);
+    }
+
+    public function promiseInterfaceMethodProvider()
+    {
+        return [
+            ['then', [null, null]],
+            ['otherwise', [function () {}]],
+            ['wait', [true]],
+            ['getState', []],
+            ['resolve', [null]],
+            ['reject', [null]],
+        ];
+    }
+
+    public function testShouldCancelResultPromiseAndOutsideCurrentPromise()
+    {
+        $coroutine = new Coroutine(function () { yield 0; });
+
+        $mockPromises = [
+            'result' => $this->getMockForAbstractClass(PromiseInterface::class),
+            'currentPromise' => $this->getMockForAbstractClass(PromiseInterface::class),
+        ];
+        foreach ($mockPromises as $propName => $mockPromise) {
+            /**
+             * @var $mockPromise \PHPUnit_Framework_MockObject_MockObject
+             */
+            $mockPromise->expects($this->once())
+                ->method('cancel')
+                ->with();
+
+            $promiseProp = (new ReflectionClass(Coroutine::class))->getProperty($propName);
+            $promiseProp->setAccessible(true);
+            $promiseProp->setValue($coroutine, $mockPromise);
+        }
+
+        $coroutine->cancel();
+    }
+}