Fix happy eyeballs races with custom resolver (#2436)

Motivation:

The HappyEyeballs connector synchronises state on an event loop but
calls out to a 'Resolver' to do DNS lookups. The resolver returns
results as a future which may be on a different loop than the connector.
The connector does not hop back to its own event loop before processing
the results.

For client bootstraps, if no resolver is specified then the default
resolver uses the same event loop as the connector so in many cases this
is not an issue. However, if a custom resolver is used this guarantee is
lost and data races are much more likely.

Modifications:

- Hop back to the connector's event loop after calling the resolver.
- Add a test.

Result:

Fewer data races.

Author: glbrntt

Sources/NIOPosix/HappyEyeballs.swift

@@ -433,8 +433,14 @@ internal class HappyEyeballsConnector {
         // The two queries SHOULD be made as soon after one another as possible,
         // with the AAAA query made first and immediately followed by the A
         // query.
-        whenAAAALookupComplete(future: resolver.initiateAAAAQuery(host: host, port: port))
-        whenALookupComplete(future: resolver.initiateAQuery(host: host, port: port))
+        //
+        // We hop back to `self.loop` because there's no guarantee the resolver runs
+        // on our event loop.
+        let aaaaLookup = self.resolver.initiateAAAAQuery(host: self.host, port: self.port).hop(to: self.loop)
+        self.whenAAAALookupComplete(future: aaaaLookup)
+
+        let aLookup = self.resolver.initiateAQuery(host: self.host, port: self.port).hop(to: self.loop)
+        self.whenALookupComplete(future: aLookup)
     }
 
     /// Called when the A query has completed before the AAAA query.

Tests/NIOPosixTests/HappyEyeballsTest.swift

@@ -1231,4 +1231,34 @@ public final class HappyEyeballsTest : XCTestCase {
             XCTAssertEqual(channel.state(), .closed)
         }
     }
+
+    func testResolverOnDifferentEventLoop() throws {
+        // Tests a regression where the happy eyeballs connector would update its state on the event
+        // loop of the future returned by the resolver (which may be different to its own). Prior
+        // to the fix this test would trigger TSAN warnings.
+        let group = MultiThreadedEventLoopGroup(numberOfThreads: 2)
+        defer {
+            XCTAssertNoThrow(try group.syncShutdownGracefully())
+        }
+
+        let server = try ServerBootstrap(group: group)
+            .bind(host: "localhost", port: 0)
+            .wait()
+
+        defer {
+            XCTAssertNoThrow(try server.close().wait())
+        }
+
+        // Run the resolver and connection on different event loops.
+        let resolverLoop = group.next()
+        let connectionLoop = group.next()
+        XCTAssertNotIdentical(resolverLoop, connectionLoop)
+        let resolver = GetaddrinfoResolver(loop: resolverLoop, aiSocktype: .stream, aiProtocol: .tcp)
+        let client = try ClientBootstrap(group: connectionLoop)
+            .resolver(resolver)
+            .connect(host: "localhost", port: server.localAddress!.port!)
+            .wait()
+
+        XCTAssertNoThrow(try client.close().wait())
+    }
 }