diff --git a/src/Query/RetryExecutor.php b/src/Query/RetryExecutor.php index 90353e5e..46e2ef96 100644 --- a/src/Query/RetryExecutor.php +++ b/src/Query/RetryExecutor.php @@ -2,6 +2,7 @@ namespace React\Dns\Query; +use React\Promise\CancellablePromiseInterface; use React\Promise\Deferred; class RetryExecutor implements ExecutorInterface @@ -22,23 +23,57 @@ public function query($nameserver, Query $query) public function tryQuery($nameserver, Query $query, $retries) { - $that = $this; - $errorback = function ($error) use ($nameserver, $query, $retries, $that) { - if (!$error instanceof TimeoutException) { - throw $error; + $deferred = new Deferred(function () use (&$promise) { + if ($promise instanceof CancellablePromiseInterface) { + $promise->cancel(); } - if (0 >= $retries) { - throw new \RuntimeException( - sprintf("DNS query for %s failed: too many retries", $query->name), + }); + + $success = function ($value) use ($deferred, &$errorback) { + $errorback = null; + $deferred->resolve($value); + }; + + $executor = $this->executor; + $errorback = function ($e) use ($deferred, &$promise, $nameserver, $query, $success, &$errorback, &$retries, $executor) { + if (!$e instanceof TimeoutException) { + $errorback = null; + $deferred->reject($e); + } elseif ($retries <= 0) { + $errorback = null; + $deferred->reject($e = new \RuntimeException( + 'DNS query for ' . $query->name . ' failed: too many retries', 0, - $error + $e + )); + + // avoid garbage references by replacing all closures in call stack. + // what a lovely piece of code! + $r = new \ReflectionProperty('Exception', 'trace'); + $r->setAccessible(true); + $trace = $r->getValue($e); + foreach ($trace as &$one) { + foreach ($one['args'] as &$arg) { + if ($arg instanceof \Closure) { + $arg = 'Object(' . \get_class($arg) . ')'; + } + } + } + $r->setValue($e, $trace); + } else { + --$retries; + $promise = $executor->query($nameserver, $query)->then( + $success, + $errorback ); } - return $that->tryQuery($nameserver, $query, $retries-1); }; - return $this->executor - ->query($nameserver, $query) - ->then(null, $errorback); + $promise = $this->executor->query($nameserver, $query)->then( + $success, + $errorback + ); + + return $deferred->promise(); } } diff --git a/tests/Query/RetryExecutorTest.php b/tests/Query/RetryExecutorTest.php index 8950f84f..7e44a084 100644 --- a/tests/Query/RetryExecutorTest.php +++ b/tests/Query/RetryExecutorTest.php @@ -162,6 +162,159 @@ public function queryShouldCancelQueryOnCancel() $this->assertEquals(1, $cancelled); } + /** + * @covers React\Dns\Query\RetryExecutor + * @test + */ + public function queryShouldCancelSecondQueryOnCancel() + { + $deferred = new Deferred(); + $cancelled = 0; + + $executor = $this->createExecutorMock(); + $executor + ->expects($this->exactly(2)) + ->method('query') + ->with('8.8.8.8', $this->isInstanceOf('React\Dns\Query\Query')) + ->will($this->onConsecutiveCalls( + $this->returnValue($deferred->promise()), + $this->returnCallback(function ($domain, $query) use (&$cancelled) { + $deferred = new Deferred(function ($resolve, $reject) use (&$cancelled) { + ++$cancelled; + $reject(new CancellationException('Cancelled')); + }); + + return $deferred->promise(); + }) + )); + + $retryExecutor = new RetryExecutor($executor, 2); + + $query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451); + $promise = $retryExecutor->query('8.8.8.8', $query); + + $promise->then($this->expectCallableNever(), $this->expectCallableOnce()); + + // first query will time out after a while and this sends the next query + $deferred->reject(new TimeoutException()); + + $this->assertEquals(0, $cancelled); + $promise->cancel(); + $this->assertEquals(1, $cancelled); + } + + /** + * @covers React\Dns\Query\RetryExecutor + * @test + */ + public function queryShouldNotCauseGarbageReferencesOnSuccess() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + $executor = $this->createExecutorMock(); + $executor + ->expects($this->once()) + ->method('query') + ->with('8.8.8.8', $this->isInstanceOf('React\Dns\Query\Query')) + ->willReturn(Promise\resolve($this->createStandardResponse())); + + $retryExecutor = new RetryExecutor($executor, 0); + + gc_collect_cycles(); + $query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451); + $retryExecutor->query('8.8.8.8', $query); + + $this->assertEquals(0, gc_collect_cycles()); + } + + /** + * @covers React\Dns\Query\RetryExecutor + * @test + */ + public function queryShouldNotCauseGarbageReferencesOnTimeoutErrors() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + $executor = $this->createExecutorMock(); + $executor + ->expects($this->any()) + ->method('query') + ->with('8.8.8.8', $this->isInstanceOf('React\Dns\Query\Query')) + ->willReturn(Promise\reject(new TimeoutException("timeout"))); + + $retryExecutor = new RetryExecutor($executor, 0); + + gc_collect_cycles(); + $query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451); + $retryExecutor->query('8.8.8.8', $query); + + $this->assertEquals(0, gc_collect_cycles()); + } + + /** + * @covers React\Dns\Query\RetryExecutor + * @test + */ + public function queryShouldNotCauseGarbageReferencesOnCancellation() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + $deferred = new Deferred(function () { + throw new \RuntimeException(); + }); + + $executor = $this->createExecutorMock(); + $executor + ->expects($this->once()) + ->method('query') + ->with('8.8.8.8', $this->isInstanceOf('React\Dns\Query\Query')) + ->willReturn($deferred->promise()); + + $retryExecutor = new RetryExecutor($executor, 0); + + gc_collect_cycles(); + $query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451); + $promise = $retryExecutor->query('8.8.8.8', $query); + $promise->cancel(); + $promise = null; + + $this->assertEquals(0, gc_collect_cycles()); + } + + /** + * @covers React\Dns\Query\RetryExecutor + * @test + */ + public function queryShouldNotCauseGarbageReferencesOnNonTimeoutErrors() + { + if (class_exists('React\Promise\When')) { + $this->markTestSkipped('Not supported on legacy Promise v1 API'); + } + + $executor = $this->createExecutorMock(); + $executor + ->expects($this->once()) + ->method('query') + ->with('8.8.8.8', $this->isInstanceOf('React\Dns\Query\Query')) + ->will($this->returnCallback(function ($domain, $query) { + return Promise\reject(new \Exception); + })); + + $retryExecutor = new RetryExecutor($executor, 2); + + gc_collect_cycles(); + $query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451); + $retryExecutor->query('8.8.8.8', $query); + + $this->assertEquals(0, gc_collect_cycles()); + } + protected function expectPromiseOnce($return = null) { $mock = $this->createPromiseMock();