Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cancelling happy eyeballs when IPv6 resolution is pending #311

Merged
merged 1 commit into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/HappyEyeBallsConnectionBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,8 @@ public function __construct(LoopInterface $loop, ConnectorInterface $connector,

public function connect()
{
$timer = null;
$that = $this;
return new Promise\Promise(function ($resolve, $reject) use ($that, &$timer) {
return new Promise\Promise(function ($resolve, $reject) use ($that) {
$lookupResolve = function ($type) use ($that, $resolve, $reject) {
return function (array $ips) use ($that, $type, $resolve, $reject) {
unset($that->resolverPromises[$type]);
Expand All @@ -83,36 +82,36 @@ public function connect()
};

$that->resolverPromises[Message::TYPE_AAAA] = $that->resolve(Message::TYPE_AAAA, $reject)->then($lookupResolve(Message::TYPE_AAAA));
$that->resolverPromises[Message::TYPE_A] = $that->resolve(Message::TYPE_A, $reject)->then(function (array $ips) use ($that, &$timer) {
$that->resolverPromises[Message::TYPE_A] = $that->resolve(Message::TYPE_A, $reject)->then(function (array $ips) use ($that) {
// happy path: IPv6 has resolved already (or could not resolve), continue with IPv4 addresses
if ($that->resolved[Message::TYPE_AAAA] === true || !$ips) {
return $ips;
}

// Otherwise delay processing IPv4 lookup until short timer passes or IPv6 resolves in the meantime
$deferred = new Promise\Deferred();
$deferred = new Promise\Deferred(function () use (&$ips) {
// discard all IPv4 addresses if cancelled
WyriHaximus marked this conversation as resolved.
Show resolved Hide resolved
$ips = array();
});
$timer = $that->loop->addTimer($that::RESOLUTION_DELAY, function () use ($deferred, $ips) {
$deferred->resolve($ips);
});

$that->resolverPromises[Message::TYPE_AAAA]->then(function () use ($that, $timer, $deferred, $ips) {
$that->resolverPromises[Message::TYPE_AAAA]->then(function () use ($that, $timer, $deferred, &$ips) {
WyriHaximus marked this conversation as resolved.
Show resolved Hide resolved
$that->loop->cancelTimer($timer);
$deferred->resolve($ips);
});

return $deferred->promise();
})->then($lookupResolve(Message::TYPE_A));
}, function ($_, $reject) use ($that, &$timer) {
}, function ($_, $reject) use ($that) {
$reject(new \RuntimeException(
'Connection to ' . $that->uri . ' cancelled' . (!$that->connectionPromises ? ' during DNS lookup' : '') . ' (ECONNABORTED)',
\defined('SOCKET_ECONNABORTED') ? \SOCKET_ECONNABORTED : 103
));
$_ = $reject = null;

$that->cleanUp();
if ($timer instanceof TimerInterface) {
$that->loop->cancelTimer($timer);
}
});
}

Expand Down Expand Up @@ -247,13 +246,15 @@ public function cleanUp()
// clear list of outstanding IPs to avoid creating new connections
$this->connectQueue = array();

// cancel pending connection attempts
foreach ($this->connectionPromises as $connectionPromise) {
if ($connectionPromise instanceof PromiseInterface && \method_exists($connectionPromise, 'cancel')) {
$connectionPromise->cancel();
}
}

foreach ($this->resolverPromises as $resolverPromise) {
// cancel pending DNS resolution (cancel IPv4 first in case it is awaiting IPv6 resolution delay)
foreach (\array_reverse($this->resolverPromises) as $resolverPromise) {
if ($resolverPromise instanceof PromiseInterface && \method_exists($resolverPromise, 'cancel')) {
$resolverPromise->cancel();
}
Expand Down
4 changes: 3 additions & 1 deletion tests/HappyEyeBallsConnectionBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,9 @@ public function testCancelConnectWillRejectPromiseAndCancelPendingIpv6LookupAndC
array('reactphp.org', Message::TYPE_AAAA),
array('reactphp.org', Message::TYPE_A)
)->willReturnOnConsecutiveCalls(
new Promise(function () { }, $this->expectCallableOnce()),
new Promise(function () { }, function () {
throw new \RuntimeException('DNS cancelled');
}),
\React\Promise\resolve(array('127.0.0.1'))
);

Expand Down