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.

Sign up for GitHub

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

Jump to bottom

Fix cancelling happy eyeballs to stop timer, fix rejection reason and simplify timer logic #225

Merged
merged 2 commits into from
Mar 11, 2020
Merged

Fix cancelling happy eyeballs to stop timer, fix rejection reason and simplify timer logic #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b400a57
Fix cancelling happy eyeballs to stop timer and fix rejection reason
clue Mar 9, 2020
511f85d
Fix resolution delay (50ms) and simplify related timer logic
clue Mar 10, 2020
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
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"php": ">=5.3.0",
"evenement/evenement": "^3.0 || ^2.0 || ^1.0",
"react/dns": "^1.1",
"react/event-loop": "^1.0 || ^0.5 || ^0.4 || ^0.3.5",
"react/event-loop": "^1.0 || ^0.5",
"react/promise": "^2.6.0 || ^1.2.1",
"react/promise-timer": "^1.4.0",
"react/stream": "^1.1"
Expand Down
84 changes: 40 additions & 44 deletions src/HappyEyeBallsConnectionBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,21 @@
*/
final class HappyEyeBallsConnectionBuilder
{
const CONNECT_INTERVAL = 0.1;
const RESOLVE_WAIT = 0.5;
/**
* As long as we haven't connected yet keep popping an IP address of the connect queue until one of them
* succeeds or they all fail. We will wait 100ms between connection attempts as per RFC.
*
* @link https://tools.ietf.org/html/rfc8305#section-5
*/
const CONNECTION_ATTEMPT_DELAY = 0.1;

/**
* Delay `A` lookup by 50ms sending out connection to IPv4 addresses when IPv6 records haven't
* resolved yet as per RFC.
*
* @link https://tools.ietf.org/html/rfc8305#section-3
*/
const RESOLUTION_DELAY = 0.05;

public $loop;
public $connector;
Expand All @@ -29,7 +42,7 @@ final class HappyEyeBallsConnectionBuilder
public $resolverPromises = array();
public $connectionPromises = array();
public $connectQueue = array();
public $timer;
public $nextAttemptTimer;
public $parts;
public $ipsCount = 0;
public $failureCount = 0;
Expand All @@ -48,66 +61,52 @@ public function __construct(LoopInterface $loop, ConnectorInterface $connector,

public function connect()
{
$timer = null;
$that = $this;
return new Promise\Promise(function ($resolve, $reject) use ($that) {
return new Promise\Promise(function ($resolve, $reject) use ($that, &$timer) {
$lookupResolve = function ($type) use ($that, $resolve, $reject) {
return function (array $ips) use ($that, $type, $resolve, $reject) {
unset($that->resolverPromises[$type]);
$that->resolved[$type] = true;

$that->mixIpsIntoConnectQueue($ips);

if ($that->timer instanceof TimerInterface) {
if ($that->nextAttemptTimer instanceof TimerInterface) {
return;
}

$that->check($resolve, $reject);
};
};

$ipv4Deferred = null;
$timer = null;
$that->resolverPromises[Message::TYPE_AAAA] = $that->resolve(Message::TYPE_AAAA, $reject)->then($lookupResolve(Message::TYPE_AAAA))->then(function () use (&$ipv4Deferred) {
if ($ipv4Deferred instanceof Promise\Deferred) {
$ipv4Deferred->resolve();
}
});
$that->resolverPromises[Message::TYPE_A] = $that->resolve(Message::TYPE_A, $reject)->then(function ($ips) use ($that, &$ipv4Deferred, &$timer) {
$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 ($ips) use ($that, &$timer) {
// happy path: IPv6 has resolved already, continue with IPv4 addresses
if ($that->resolved[Message::TYPE_AAAA] === true) {
return Promise\resolve($ips);
return $ips;
}

/**
* Delay A lookup by 50ms sending out connection to IPv4 addresses when IPv6 records haven't
* resolved yet as per RFC.
*
* @link https://tools.ietf.org/html/rfc8305#section-3
*/
$ipv4Deferred = new Promise\Deferred();
// Otherwise delay processing IPv4 lookup until short timer passes or IPv6 resolves in the meantime
$deferred = new Promise\Deferred();

$timer = $that->loop->addTimer($that::RESOLVE_WAIT, function () use ($deferred, $ips) {
$ipv4Deferred = null;
$timer = $that->loop->addTimer($that::RESOLUTION_DELAY, function () use ($deferred, $ips) {
$deferred->resolve($ips);
});

$ipv4Deferred->promise()->then(function () use ($that, &$timer, $deferred, $ips) {
$that->resolverPromises[Message::TYPE_AAAA]->then(function () use ($that, $timer, $deferred, $ips) {
$that->loop->cancelTimer($timer);
$deferred->resolve($ips);
});

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

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

$reject(new \RuntimeException('Connection to ' . $that->uri . ' cancelled during DNS lookup'));

$_ = $reject = null;
});
}

Expand All @@ -126,7 +125,6 @@ public function resolve($type, $reject)
}

if ($that->ipsCount === 0) {
$that->resolved = null;
$that->resolverPromises = null;
$reject(new \RuntimeException('Connection to ' . $that->uri . ' failed during DNS lookup: DNS error'));
}
Expand All @@ -138,9 +136,9 @@ public function resolve($type, $reject)
*/
public function check($resolve, $reject)
{
if (\count($this->connectQueue) === 0 && $this->resolved[Message::TYPE_A] === true && $this->resolved[Message::TYPE_AAAA] === true && $this->timer instanceof TimerInterface) {
$this->loop->cancelTimer($this->timer);
$this->timer = null;
if (\count($this->connectQueue) === 0 && $this->resolved[Message::TYPE_A] === true && $this->resolved[Message::TYPE_AAAA] === true && $this->nextAttemptTimer instanceof TimerInterface) {
$this->loop->cancelTimer($this->nextAttemptTimer);
$this->nextAttemptTimer = null;
}

if (\count($this->connectQueue) === 0) {
Expand All @@ -156,7 +154,7 @@ public function check($resolve, $reject)
$that->cleanUp();

$resolve($connection);
}, function () use ($that, $ip, $resolve, $reject) {
}, function () use ($that, $ip, $reject) {
unset($that->connectionPromises[$ip]);

$that->failureCount++;
Expand All @@ -178,8 +176,8 @@ public function check($resolve, $reject)
*
* @link https://tools.ietf.org/html/rfc8305#section-5
*/
if ((\count($this->connectQueue) > 0 || ($this->resolved[Message::TYPE_A] === false || $this->resolved[Message::TYPE_AAAA] === false)) && $this->timer === null) {
$this->timer = $this->loop->addPeriodicTimer(self::CONNECT_INTERVAL, function () use ($that, $resolve, $reject) {
if ((\count($this->connectQueue) > 0 || ($this->resolved[Message::TYPE_A] === false || $this->resolved[Message::TYPE_AAAA] === false)) && $this->nextAttemptTimer === null) {
$this->nextAttemptTimer = $this->loop->addPeriodicTimer(self::CONNECTION_ATTEMPT_DELAY, function () use ($that, $resolve, $reject) {
$that->check($resolve, $reject);
});
}
Expand Down Expand Up @@ -240,23 +238,21 @@ public function attemptConnection($ip)
*/
public function cleanUp()
{
/** @var CancellablePromiseInterface $promise */
foreach ($this->connectionPromises as $index => $connectionPromise) {
foreach ($this->connectionPromises as $connectionPromise) {
if ($connectionPromise instanceof CancellablePromiseInterface) {
$connectionPromise->cancel();
}
}

/** @var CancellablePromiseInterface $promise */
foreach ($this->resolverPromises as $index => $resolverPromise) {
foreach ($this->resolverPromises as $resolverPromise) {
if ($resolverPromise instanceof CancellablePromiseInterface) {
$resolverPromise->cancel();
}
}

if ($this->timer instanceof TimerInterface) {
$this->loop->cancelTimer($this->timer);
$this->timer = null;
if ($this->nextAttemptTimer instanceof TimerInterface) {
$this->loop->cancelTimer($this->nextAttemptTimer);
$this->nextAttemptTimer = null;
}
}

Expand Down
48 changes: 25 additions & 23 deletions tests/FunctionalConnectorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,16 @@ public function connectionToRemoteTCP4n6ServerShouldResultInOurIP()
*/
public function connectionToRemoteTCP4ServerShouldResultInOurIP()
{
if ($this->ipv4() === false) {
$this->markTestSkipped('IPv4 connection not supported on this system');
}

$loop = Factory::create();

$connector = new Connector($loop, array('happy_eyeballs' => true));

$ip = Block\await($this->request('ipv4.tlund.se', $connector), $loop, self::TIMEOUT);
try {
$ip = Block\await($this->request('ipv4.tlund.se', $connector), $loop, self::TIMEOUT);
} catch (\Exception $e) {
$this->checkIpv4();
throw $e;
}

$this->assertSame($ip, filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4), $ip);
$this->assertFalse(filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6), $ip);
Expand All @@ -76,15 +77,16 @@ public function connectionToRemoteTCP4ServerShouldResultInOurIP()
*/
public function connectionToRemoteTCP6ServerShouldResultInOurIP()
{
if ($this->ipv6() === false) {
$this->markTestSkipped('IPv6 connection not supported on this system');
}

$loop = Factory::create();

$connector = new Connector($loop, array('happy_eyeballs' => true));

$ip = Block\await($this->request('ipv6.tlund.se', $connector), $loop, self::TIMEOUT);
try {
$ip = Block\await($this->request('ipv6.tlund.se', $connector), $loop, self::TIMEOUT);
} catch (\Exception $e) {
$this->checkIpv6();
throw $e;
}

$this->assertFalse(filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4), $ip);
$this->assertSame($ip, filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6), $ip);
Expand All @@ -105,33 +107,33 @@ private function request($host, ConnectorInterface $connector)
{
$that = $this;
return $connector->connect($host . ':80')->then(function (ConnectionInterface $connection) use ($host) {
$connection->write("GET / HTTP/1.1\r\nHost: " . $host . "\r\n\r\n");
$connection->write("GET / HTTP/1.1\r\nHost: " . $host . "\r\nConnection: close\r\n\r\n");

return \React\Promise\Stream\buffer($connection);
})->then(function ($response) use ($that) {
return $that->parseIpFromPage($response);
});
}

private function ipv4()
private function checkIpv4()
{
if ($this->ipv4 !== null) {
return $this->ipv4;
if ($this->ipv4 === null) {
$this->ipv4 = !!@file_get_contents('http://ipv4.tlund.se/');
}

$this->ipv4 = !!@file_get_contents('http://ipv4.tlund.se/');

return $this->ipv4;
if (!$this->ipv4) {
$this->markTestSkipped('IPv4 connection not supported on this system');
}
}

private function ipv6()
private function checkIpv6()
{
if ($this->ipv6 !== null) {
return $this->ipv6;
if ($this->ipv6 === null) {
$this->ipv6 = !!@file_get_contents('http://ipv6.tlund.se/');
}

$this->ipv6 = !!@file_get_contents('http://ipv6.tlund.se/');

return $this->ipv6;
if (!$this->ipv6) {
$this->markTestSkipped('IPv6 connection not supported on this system');
}
}
}
Loading