Fix bug in migration from HTTP1 to HTTP2 and back to HTTP1 (#486)

Author: dnadoba

Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift

@@ -532,12 +532,21 @@ extension HTTPConnectionPool {
 
         mutating func migrateToHTTP2() -> HTTP1ToHTTP2MigrationContext {
             var migrationContext = HTTP1ToHTTP2MigrationContext()
-            self.connections.removeAll { connection in
-                switch connection.migrateToHTTP2(&migrationContext) {
+            let initialOverflowIndex = self.overflowIndex
+
+            self.connections = self.connections.enumerated().compactMap { index, connectionState in
+                switch connectionState.migrateToHTTP2(&migrationContext) {
                 case .removeConnection:
-                    return true
+                    // If the connection has an index smaller than the previous overflow index,
+                    // we deal with a general purpose connection.
+                    // For this reason we need to decrement the overflow index.
+                    if index < initialOverflowIndex {
+                        self.overflowIndex = self.connections.index(before: self.overflowIndex)
+                    }
+                    return nil
+
                 case .keepConnection:
-                    return false
+                    return connectionState
                 }
             }
             return migrationContext
@@ -654,6 +663,9 @@ extension HTTPConnectionPool {
             self.connections = self.connections.enumerated().compactMap { index, connectionState in
                 switch connectionState.cleanup(&cleanupContext) {
                 case .removeConnection:
+                    // If the connection has an index smaller than the previous overflow index,
+                    // we deal with a general purpose connection.
+                    // For this reason we need to decrement the overflow index.
                     if index < initialOverflowIndex {
                         self.overflowIndex = self.connections.index(before: self.overflowIndex)
                     }

Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest+XCTest.swift

@@ -41,6 +41,7 @@ extension HTTPConnectionPool_HTTP1ConnectionsTests {
             ("testMigrationFromHTTP2WithAlreadyLeasedHTTP1Connection", testMigrationFromHTTP2WithAlreadyLeasedHTTP1Connection),
             ("testMigrationFromHTTP2WithMoreStartingConnectionsThanMaximumAllowedConccurentConnections", testMigrationFromHTTP2WithMoreStartingConnectionsThanMaximumAllowedConccurentConnections),
             ("testMigrationFromHTTP2StartsEnoghOverflowConnectionsForRequiredEventLoopRequests", testMigrationFromHTTP2StartsEnoghOverflowConnectionsForRequiredEventLoopRequests),
+            ("testMigrationFromHTTP1ToHTTP2AndBackToHTTP1", testMigrationFromHTTP1ToHTTP2AndBackToHTTP1),
         ]
     }
 }

Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift

@@ -563,4 +563,43 @@ class HTTPConnectionPool_HTTP1ConnectionsTests: XCTestCase {
             XCTAssertTrue(context.eventLoop === el)
         }
     }
+
+    func testMigrationFromHTTP1ToHTTP2AndBackToHTTP1() throws {
+        let elg = EmbeddedEventLoopGroup(loops: 2)
+        defer { XCTAssertNoThrow(try elg.syncShutdownGracefully()) }
+        let el1 = elg.next()
+        let el2 = elg.next()
+
+        let generator = HTTPConnectionPool.Connection.ID.Generator()
+        var connections = HTTPConnectionPool.HTTP1Connections(maximumConcurrentConnections: 8, generator: generator)
+
+        let connID1 = connections.createNewConnection(on: el1)
+
+        let context = connections.migrateToHTTP2()
+        XCTAssertEqual(context, .init(
+            backingOff: [],
+            starting: [(connID1, el1)],
+            close: []
+        ))
+
+        let connID2 = generator.next()
+
+        connections.migrateFromHTTP2(
+            starting: [(connID2, el2)],
+            backingOff: []
+        )
+
+        let conn2: HTTPConnectionPool.Connection = .__testOnly_connection(id: connID2, eventLoop: el2)
+        let (_, idleContext) = connections.newHTTP1ConnectionEstablished(conn2)
+        XCTAssertEqual(idleContext.use, .generalPurpose)
+        XCTAssertEqual(idleContext.eventLoop.id, el2.id)
+    }
+}
+
+extension HTTPConnectionPool.HTTP1Connections.HTTP1ToHTTP2MigrationContext: Equatable {
+    public static func == (lhs: Self, rhs: Self) -> Bool {
+        return lhs.close == rhs.close &&
+            lhs.starting.elementsEqual(rhs.starting, by: { $0.0 == $1.0 && $0.1 === $1.1 }) &&
+            lhs.backingOff.elementsEqual(rhs.backingOff, by: { $0.0 == $1.0 && $0.1 === $1.1 })
+    }
 }