diff --git a/composer.json b/composer.json index e425dc6..283685f 100644 --- a/composer.json +++ b/composer.json @@ -20,7 +20,7 @@ "require": { "php": ">=5.3", "react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5", - "react/promise": "~2.1|~1.2" + "react/promise": "2.x-dev as 2.6.0 || ^2.6.0 || ^1.2.1" }, "require-dev": { "phpunit/phpunit": "^6.4 || ^5.7 || ^4.8.35" diff --git a/src/functions.php b/src/functions.php index d59d236..070dbdb 100644 --- a/src/functions.php +++ b/src/functions.php @@ -13,7 +13,12 @@ function timeout(PromiseInterface $promise, $time, LoopInterface $loop) // thus leaving responsibility to the input promise. $canceller = null; if ($promise instanceof CancellablePromiseInterface) { - $canceller = array($promise, 'cancel'); + // pass promise by reference to clean reference after cancellation handler + // has been invoked once in order to avoid garbage references in call stack. + $canceller = function () use (&$promise) { + $promise->cancel(); + $promise = null; + }; } return new Promise(function ($resolve, $reject) use ($loop, $time, $promise) { @@ -38,12 +43,15 @@ function timeout(PromiseInterface $promise, $time, LoopInterface $loop) } // start timeout timer which will cancel the input promise - $timer = $loop->addTimer($time, function () use ($time, $promise, $reject) { + $timer = $loop->addTimer($time, function () use ($time, &$promise, $reject) { $reject(new TimeoutException($time, 'Timed out after ' . $time . ' seconds')); + // try to invoke cancellation handler of input promise and then clean + // reference in order to avoid garbage references in call stack. if ($promise instanceof CancellablePromiseInterface) { $promise->cancel(); } + $promise = null; }); }, $canceller); } @@ -55,11 +63,12 @@ function resolve($time, LoopInterface $loop) $timer = $loop->addTimer($time, function () use ($time, $resolve) { $resolve($time); }); - }, function ($resolve, $reject, $notify) use (&$timer, $loop) { - // cancelling this promise will cancel the timer and reject + }, function () use (&$timer, $loop) { + // cancelling this promise will cancel the timer, clean the reference + // in order to avoid garbage references in call stack and then reject. $loop->cancelTimer($timer); + $timer = null; - $resolve = $reject = $notify = $timer = $loop = null; throw new \RuntimeException('Timer cancelled'); }); } diff --git a/tests/FunctionRejectTest.php b/tests/FunctionRejectTest.php index 6153fcc..dbbff3d 100644 --- a/tests/FunctionRejectTest.php +++ b/tests/FunctionRejectTest.php @@ -38,7 +38,7 @@ public function testPromiseExpiredWillBeRejectedOnTimeout() $this->expectPromiseRejected($promise); } - public function testCancelingPromiseWillRejectTimer() + public function testCancellingPromiseWillRejectTimer() { $promise = Timer\reject(0.01, $this->loop); @@ -46,4 +46,34 @@ public function testCancelingPromiseWillRejectTimer() $this->expectPromiseRejected($promise); } + + public function testWaitingForPromiseToRejectDoesNotLeaveGarbageCycles() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $promise = Timer\reject(0.01, $this->loop); + $this->loop->run(); + unset($promise); + + $this->assertEquals(0, gc_collect_cycles()); + } + + public function testCancellingPromiseDoesNotLeaveGarbageCycles() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $promise = Timer\reject(0.01, $this->loop); + $promise->cancel(); + unset($promise); + + $this->assertEquals(0, gc_collect_cycles()); + } } diff --git a/tests/FunctionResolveTest.php b/tests/FunctionResolveTest.php index 0bfdc21..c4e2be7 100644 --- a/tests/FunctionResolveTest.php +++ b/tests/FunctionResolveTest.php @@ -60,7 +60,7 @@ public function testCancellingPromiseWillCancelLoopTimer() $promise->cancel(); } - public function testCancelingPromiseWillRejectTimer() + public function testCancellingPromiseWillRejectTimer() { $promise = Timer\resolve(0.01, $this->loop); @@ -68,4 +68,34 @@ public function testCancelingPromiseWillRejectTimer() $this->expectPromiseRejected($promise); } + + public function testWaitingForPromiseToResolveDoesNotLeaveGarbageCycles() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $promise = Timer\resolve(0.01, $this->loop); + $this->loop->run(); + unset($promise); + + $this->assertEquals(0, gc_collect_cycles()); + } + + public function testCancellingPromiseDoesNotLeaveGarbageCycles() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $promise = Timer\resolve(0.01, $this->loop); + $promise->cancel(); + unset($promise); + + $this->assertEquals(0, gc_collect_cycles()); + } } diff --git a/tests/FunctionTimeoutTest.php b/tests/FunctionTimeoutTest.php index aaca2da..1fd705f 100644 --- a/tests/FunctionTimeoutTest.php +++ b/tests/FunctionTimeoutTest.php @@ -166,4 +166,80 @@ public function testCancelTimeoutWillResolveIfGivenPromiseWillResolve() $this->expectPromiseResolved($promise); $this->expectPromiseResolved($timeout); } + + public function testWaitingForPromiseToResolveBeforeTimeoutDoesNotLeaveGarbageCycles() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $promise = Timer\resolve(0.01, $this->loop); + + $promise = Timer\timeout($promise, 1.0, $this->loop); + + $this->loop->run(); + unset($promise); + + $this->assertEquals(0, gc_collect_cycles()); + } + + public function testWaitingForPromiseToRejectBeforeTimeoutDoesNotLeaveGarbageCycles() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $promise = Timer\reject(0.01, $this->loop); + + $promise = Timer\timeout($promise, 1.0, $this->loop); + + $this->loop->run(); + unset($promise); + + $this->assertEquals(0, gc_collect_cycles()); + } + + public function testWaitingForPromiseToTimeoutDoesNotLeaveGarbageCycles() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $promise = new \React\Promise\Promise(function () { }, function () { + throw new \RuntimeException(); + }); + + $promise = Timer\timeout($promise, 0.01, $this->loop); + + $this->loop->run(); + unset($promise); + + $this->assertEquals(0, gc_collect_cycles()); + } + + public function testCancellingPromiseDoesNotLeaveGarbageCycles() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + gc_collect_cycles(); + + $promise = new \React\Promise\Promise(function () { }, function () { + throw new \RuntimeException(); + }); + + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $promise = Timer\timeout($promise, 0.01, $loop); + $promise->cancel(); + unset($promise); + + $this->assertEquals(0, gc_collect_cycles()); + } }