From 584408165baf87504c2e40d1b6bbbed7e2946008 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Fri, 29 Jun 2018 19:19:58 +0200 Subject: [PATCH 1/3] test against memory leak in Promise::fromObservable --- .../Functional/React/PromiseFromObservableTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/Rx/Functional/React/PromiseFromObservableTest.php b/test/Rx/Functional/React/PromiseFromObservableTest.php index 88829fed..198116fb 100644 --- a/test/Rx/Functional/React/PromiseFromObservableTest.php +++ b/test/Rx/Functional/React/PromiseFromObservableTest.php @@ -49,4 +49,18 @@ function ($error) { $this->assertEquals($error, new Exception("some error")); }); } + + /** + * @test + */ + public function promise_no_memory_leak() + { + gc_collect_cycles(); + + $source = Observable::of(42); + + Promise::fromObservable($source)->done(); + + $this->assertSame(0, gc_collect_cycles()); + } } From 29660b0c03c766ec1ead196a90d0caf1fbb2c808 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Fri, 29 Jun 2018 19:25:21 +0200 Subject: [PATCH 2/3] Reworked Promise::fromObservable to leave less cyclomatic references around --- src/React/Promise.php | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/React/Promise.php b/src/React/Promise.php index 9b2c0881..29fcf97a 100644 --- a/src/React/Promise.php +++ b/src/React/Promise.php @@ -46,26 +46,32 @@ public static function rejected($exception): ReactPromise */ public static function fromObservable(ObservableInterface $observable, Deferred $deferred = null): ReactPromise { + $subscription = null; + $promise = new ReactPromise(static function ($resolve, $reject) use (&$subscription, $observable) { + $value = null; + $subscription = $observable->subscribe( + static function ($v) use (&$value) { + $value = $v; + }, + static function ($error) use ($reject) { + $reject($error); + }, + static function () use ($resolve, &$value) { + $resolve($value); + } + ); - $d = $deferred ?: new Deferred(function () use (&$subscription) { + }, static function () use (&$subscription) { $subscription->dispose(); }); - $value = null; + if ($deferred === null) { + return $promise; + } - $subscription = $observable->subscribe( - function ($v) use (&$value) { - $value = $v; - }, - function ($error) use ($d) { - $d->reject($error); - }, - function () use ($d, &$value) { - $d->resolve($value); - } - ); + $promise->done([$deferred, 'resolve'], [$deferred, 'reject']); - return $d->promise(); + return $deferred->promise(); } /** From 6e81d33c0b76ce749532cb3b3ab33ecc370112d8 Mon Sep 17 00:00:00 2001 From: Cees-Jan Kiewiet Date: Sat, 30 Jun 2018 22:33:35 +0200 Subject: [PATCH 3/3] Promise::fromObservable test with passing a Deferred --- .../Functional/React/PromiseFromObservableTest.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/Rx/Functional/React/PromiseFromObservableTest.php b/test/Rx/Functional/React/PromiseFromObservableTest.php index 198116fb..4c08f82c 100644 --- a/test/Rx/Functional/React/PromiseFromObservableTest.php +++ b/test/Rx/Functional/React/PromiseFromObservableTest.php @@ -4,6 +4,7 @@ namespace Rx\Functional\React; use Exception; +use React\Promise\Deferred; use Rx\Functional\FunctionalTestCase; use Rx\Observable; use Rx\React\Promise; @@ -61,6 +62,19 @@ public function promise_no_memory_leak() Promise::fromObservable($source)->done(); + $this->assertSame(0, gc_collect_cycles()); + } + /** + * @test + */ + public function promise_no_memory_leak_deferred() + { + gc_collect_cycles(); + + $source = Observable::of(42); + + Promise::fromObservable($source, new Deferred())->done(); + $this->assertSame(0, gc_collect_cycles()); } }