Skip to content

Commit 4e341a2

Browse files
clueclue
committed
Fix resolution delay (50ms) and simplify related timer logic
1 parent 7b1ea1a commit 4e341a2

File tree

2 files changed

+59
-56
lines changed

2 files changed

+59
-56
lines changed

‎src/HappyEyeBallsConnectionBuilder.php

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,21 @@
1414
*/
1515
final class HappyEyeBallsConnectionBuilder
1616
{
17-
const CONNECT_INTERVAL = 0.1;
18-
const RESOLVE_WAIT = 0.5;
17+
/**
18+
* As long as we haven't connected yet keep popping an IP address of the connect queue until one of them
19+
* succeeds or they all fail. We will wait 100ms between connection attempts as per RFC.
20+
*
21+
* @link https://tools.ietf.org/html/rfc8305#section-5
22+
*/
23+
const CONNECTION_ATTEMPT_DELAY = 0.1;
24+
25+
/**
26+
* Delay `A` lookup by 50ms sending out connection to IPv4 addresses when IPv6 records haven't
27+
* resolved yet as per RFC.
28+
*
29+
* @link https://tools.ietf.org/html/rfc8305#section-3
30+
*/
31+
const RESOLUTION_DELAY = 0.05;
1932

2033
public $loop;
2134
public $connector;
@@ -29,7 +42,7 @@ final class HappyEyeBallsConnectionBuilder
2942
public $resolverPromises = array();
3043
public $connectionPromises = array();
3144
public $connectQueue = array();
32-
public $timer;
45+
public $nextAttemptTimer;
3346
public $parts;
3447
public $ipsCount = 0;
3548
public $failureCount = 0;
@@ -58,40 +71,28 @@ public function connect()
5871

5972
$that->mixIpsIntoConnectQueue($ips);
6073

61-
if ($that->timer instanceof TimerInterface) {
74+
if ($that->nextAttemptTimer instanceof TimerInterface) {
6275
return;
6376
}
6477

6578
$that->check($resolve, $reject);
6679
};
6780
};
6881

69-
$ipv4Deferred = null;
70-
$that->resolverPromises[Message::TYPE_AAAA] = $that->resolve(Message::TYPE_AAAA, $reject)->then($lookupResolve(Message::TYPE_AAAA))->then(function () use (&$ipv4Deferred) {
71-
if ($ipv4Deferred instanceof Promise\Deferred) {
72-
$ipv4Deferred->resolve();
73-
}
74-
});
75-
$that->resolverPromises[Message::TYPE_A] = $that->resolve(Message::TYPE_A, $reject)->then(function ($ips) use ($that, &$ipv4Deferred, &$timer) {
82+
$that->resolverPromises[Message::TYPE_AAAA] = $that->resolve(Message::TYPE_AAAA, $reject)->then($lookupResolve(Message::TYPE_AAAA));
83+
$that->resolverPromises[Message::TYPE_A] = $that->resolve(Message::TYPE_A, $reject)->then(function ($ips) use ($that, &$timer) {
84+
// happy path: IPv6 has resolved already, continue with IPv4 addresses
7685
if ($that->resolved[Message::TYPE_AAAA] === true) {
77-
return Promise\resolve($ips);
86+
return $ips;
7887
}
7988

80-
/**
81-
* Delay A lookup by 50ms sending out connection to IPv4 addresses when IPv6 records haven't
82-
* resolved yet as per RFC.
83-
*
84-
* @link https://tools.ietf.org/html/rfc8305#section-3
85-
*/
86-
$ipv4Deferred = new Promise\Deferred();
89+
// Otherwise delay processing IPv4 lookup until short timer passes or IPv6 resolves in the meantime
8790
$deferred = new Promise\Deferred();
88-
89-
$timer = $that->loop->addTimer($that::RESOLVE_WAIT, function () use ($deferred, $ips) {
90-
$ipv4Deferred = null;
91+
$timer = $that->loop->addTimer($that::RESOLUTION_DELAY, function () use ($deferred, $ips) {
9192
$deferred->resolve($ips);
9293
});
9394

94-
$ipv4Deferred->promise()->then(function () use ($that, &$timer, $deferred, $ips) {
95+
$that->resolverPromises[Message::TYPE_AAAA]->then(function () use ($that, $timer, $deferred, $ips) {
9596
$that->loop->cancelTimer($timer);
9697
$deferred->resolve($ips);
9798
});
@@ -136,9 +137,9 @@ public function resolve($type, $reject)
136137
*/
137138
public function check($resolve, $reject)
138139
{
139-
if (\count($this->connectQueue) === 0 && $this->resolved[Message::TYPE_A] === true && $this->resolved[Message::TYPE_AAAA] === true && $this->timer instanceof TimerInterface) {
140-
$this->loop->cancelTimer($this->timer);
141-
$this->timer = null;
140+
if (\count($this->connectQueue) === 0 && $this->resolved[Message::TYPE_A] === true && $this->resolved[Message::TYPE_AAAA] === true && $this->nextAttemptTimer instanceof TimerInterface) {
141+
$this->loop->cancelTimer($this->nextAttemptTimer);
142+
$this->nextAttemptTimer = null;
142143
}
143144

144145
if (\count($this->connectQueue) === 0) {
@@ -176,8 +177,8 @@ public function check($resolve, $reject)
176177
*
177178
* @link https://tools.ietf.org/html/rfc8305#section-5
178179
*/
179-
if ((\count($this->connectQueue) > 0 || ($this->resolved[Message::TYPE_A] === false || $this->resolved[Message::TYPE_AAAA] === false)) && $this->timer === null) {
180-
$this->timer = $this->loop->addPeriodicTimer(self::CONNECT_INTERVAL, function () use ($that, $resolve, $reject) {
180+
if ((\count($this->connectQueue) > 0 || ($this->resolved[Message::TYPE_A] === false || $this->resolved[Message::TYPE_AAAA] === false)) && $this->nextAttemptTimer === null) {
181+
$this->nextAttemptTimer = $this->loop->addPeriodicTimer(self::CONNECTION_ATTEMPT_DELAY, function () use ($that, $resolve, $reject) {
181182
$that->check($resolve, $reject);
182183
});
183184
}
@@ -239,22 +240,22 @@ public function attemptConnection($ip)
239240
public function cleanUp()
240241
{
241242
/** @var CancellablePromiseInterface $promise */
242-
foreach ($this->connectionPromises as $index => $connectionPromise) {
243+
foreach ($this->connectionPromises as $connectionPromise) {
243244
if ($connectionPromise instanceof CancellablePromiseInterface) {
244245
$connectionPromise->cancel();
245246
}
246247
}
247248

248249
/** @var CancellablePromiseInterface $promise */
249-
foreach ($this->resolverPromises as $index => $resolverPromise) {
250+
foreach ($this->resolverPromises as $resolverPromise) {
250251
if ($resolverPromise instanceof CancellablePromiseInterface) {
251252
$resolverPromise->cancel();
252253
}
253254
}
254255

255-
if ($this->timer instanceof TimerInterface) {
256-
$this->loop->cancelTimer($this->timer);
257-
$this->timer = null;
256+
if ($this->nextAttemptTimer instanceof TimerInterface) {
257+
$this->loop->cancelTimer($this->nextAttemptTimer);
258+
$this->nextAttemptTimer = null;
258259
}
259260
}
260261

‎tests/FunctionalConnectorTest.php

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,16 @@ public function connectionToRemoteTCP4n6ServerShouldResultInOurIP()
5656
*/
5757
public function connectionToRemoteTCP4ServerShouldResultInOurIP()
5858
{
59-
if ($this->ipv4() === false) {
60-
$this->markTestSkipped('IPv4 connection not supported on this system');
61-
}
62-
6359
$loop = Factory::create();
6460

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

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

6970
$this->assertSame($ip, filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4), $ip);
7071
$this->assertFalse(filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6), $ip);
@@ -76,15 +77,16 @@ public function connectionToRemoteTCP4ServerShouldResultInOurIP()
7677
*/
7778
public function connectionToRemoteTCP6ServerShouldResultInOurIP()
7879
{
79-
if ($this->ipv6() === false) {
80-
$this->markTestSkipped('IPv6 connection not supported on this system');
81-
}
82-
8380
$loop = Factory::create();
8481

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

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

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

110112
return \React\Promise\Stream\buffer($connection);
111113
})->then(function ($response) use ($that) {
112114
return $that->parseIpFromPage($response);
113115
});
114116
}
115117

116-
private function ipv4()
118+
private function checkIpv4()
117119
{
118-
if ($this->ipv4 !== null) {
119-
return $this->ipv4;
120+
if ($this->ipv4 === null) {
121+
$this->ipv4 = !!@file_get_contents('http://ipv4.tlund.se/');
120122
}
121123

122-
$this->ipv4 = !!@file_get_contents('http://ipv4.tlund.se/');
123-
124-
return $this->ipv4;
124+
if (!$this->ipv4) {
125+
$this->markTestSkipped('IPv4 connection not supported on this system');
126+
}
125127
}
126128

127-
private function ipv6()
129+
private function checkIpv6()
128130
{
129-
if ($this->ipv6 !== null) {
130-
return $this->ipv6;
131+
if ($this->ipv6 === null) {
132+
$this->ipv6 = !!@file_get_contents('http://ipv6.tlund.se/');
131133
}
132134

133-
$this->ipv6 = !!@file_get_contents('http://ipv6.tlund.se/');
134-
135-
return $this->ipv6;
135+
if (!$this->ipv6) {
136+
$this->markTestSkipped('IPv6 connection not supported on this system');
137+
}
136138
}
137139
}

0 commit comments

Comments
 (0)