Merge pull request #311 from clue-labs/cancel-happy

Fix cancelling happy eyeballs when IPv6 resolution is pending
reactphp · Dec 13, 2023 · ab5e7d1 · ab5e7d1
2 parents 0cd247c + 3f4a3c8
commit ab5e7d1
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 11 deletions.
diff --git a/src/HappyEyeBallsConnectionBuilder.php b/src/HappyEyeBallsConnectionBuilder.php
@@ -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]);
@@ -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
+                    $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) {
                     $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);
-            }
         });
     }
 
@@ -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();
             }

diff --git a/tests/HappyEyeBallsConnectionBuilderTest.php b/tests/HappyEyeBallsConnectionBuilderTest.php
@@ -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'))
         );