Fix possible race condition when connector settles immediately

Author: clue

src/HappyEyeBallsConnectionBuilder.php

@@ -146,7 +146,8 @@ public function check($resolve, $reject)
         $ip = \array_shift($this->connectQueue);
 
         $that = $this;
-        $that->connectionPromises[$ip] = $this->attemptConnection($ip)->then(function ($connection) use ($that, $ip, $resolve) {
+        $that->connectionPromises[$ip] = $this->attemptConnection($ip);
+        $that->connectionPromises[$ip]->then(function ($connection) use ($that, $ip, $resolve) {
             unset($that->connectionPromises[$ip]);
 
             $that->cleanUp();
@@ -180,7 +181,7 @@ public function check($resolve, $reject)
 
         // Allow next connection attempt in 100ms: https://tools.ietf.org/html/rfc8305#section-5
         // Only start timer when more IPs are queued or when DNS query is still pending (might add more IPs)
-        if (\count($this->connectQueue) > 0 || $this->resolved[Message::TYPE_A] === false || $this->resolved[Message::TYPE_AAAA] === false) {
+        if ($this->nextAttemptTimer === null && (\count($this->connectQueue) > 0 || $this->resolved[Message::TYPE_A] === false || $this->resolved[Message::TYPE_AAAA] === false)) {
             $this->nextAttemptTimer = $this->loop->addTimer(self::CONNECTION_ATTEMPT_DELAY, function () use ($that, $resolve, $reject) {
                 $that->nextAttemptTimer = null;
 

tests/HappyEyeBallsConnectionBuilderTest.php

@@ -261,13 +261,12 @@ public function testConnectWillStartConnectingWithAttemptTimerWhenOnlyIpv6Resolv
         $loop->expects($this->once())->method('addTimer')->with(0.1, $this->anything())->willReturn($timer);
         $loop->expects($this->once())->method('cancelTimer')->with($timer);
 
-        $deferred = new Deferred();
         $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
         $connector->expects($this->exactly(2))->method('connect')->withConsecutive(
             array('tcp://[::1]:80?hostname=reactphp.org'),
             array('tcp://[::2]:80?hostname=reactphp.org')
         )->willReturnOnConsecutiveCalls(
-            $deferred->promise(),
+            \React\Promise\reject(new \RuntimeException()),
             new Promise(function () { })
         );
 
@@ -287,8 +286,6 @@ public function testConnectWillStartConnectingWithAttemptTimerWhenOnlyIpv6Resolv
         $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);
 
         $builder->connect();
-
-        $deferred->reject(new \RuntimeException());
     }
 
     public function testConnectWillStartConnectingAndWillStartNextConnectionWithoutNewAttemptTimerWhenNextAttemptTimerFiresAfterIpv4Rejected()
@@ -566,4 +563,33 @@ public function testAttemptConnectionWillConnectViaConnectorToGivenIpv6WithAllUr
 
         $builder->attemptConnection('::1');
     }
+
+    public function testCheckCallsRejectFunctionImmediateWithoutLeavingDanglingPromiseWhenConnectorRejectsImmediately()
+    {
+        $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
+
+        $connector = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
+        $connector->expects($this->once())->method('connect')->with('tcp://[::1]:80/path?test=yes&hostname=reactphp.org#start')->willReturn(\React\Promise\reject(new \RuntimeException()));
+
+        $resolver = $this->getMockBuilder('React\Dns\Resolver\ResolverInterface')->getMock();
+        $resolver->expects($this->never())->method('resolveAll');
+
+        $uri = 'tcp://reactphp.org:80/path?test=yes#start';
+        $host = 'reactphp.org';
+        $parts = parse_url($uri);
+
+        $builder = new HappyEyeBallsConnectionBuilder($loop, $connector, $resolver, $uri, $host, $parts);
+
+        $ref = new \ReflectionProperty($builder, 'connectQueue');
+        $ref->setAccessible(true);
+        $ref->setValue($builder, array('::1'));
+
+        $builder->check($this->expectCallableNever(), function () { });
+
+        $ref = new \ReflectionProperty($builder, 'connectionPromises');
+        $ref->setAccessible(true);
+        $promises = $ref->getValue($builder);
+
+        $this->assertEquals(array(), $promises);
+    }
 }