diff --git a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift index 17582293d..441457253 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift @@ -168,16 +168,25 @@ extension HTTPConnectionPool { } mutating func connectionCreationBackoffDone(_ connectionID: Connection.ID) -> Action { - assert(self.connections.stats.backingOff >= 1, "At least this connection is currently in backoff") - // The naming of `failConnection` is a little confusing here. All it does is moving the - // connection state from `.backingOff` to `.closed` here. It also returns the - // connection's index. - guard let (index, context) = self.connections.failConnection(connectionID) else { - preconditionFailure("Backing off a connection that is unknown to us?") + switch self.state { + case .running: + // The naming of `failConnection` is a little confusing here. All it does is moving the + // connection state from `.backingOff` to `.closed` here. It also returns the + // connection's index. + guard let (index, context) = self.connections.failConnection(connectionID) else { + preconditionFailure("Backing off a connection that is unknown to us?") + } + // In `nextActionForFailedConnection` a decision will be made whether the failed + // connection should be replaced or removed. + return self.nextActionForFailedConnection(at: index, context: context) + + case .shuttingDown, .shutDown: + // There might be a race between shutdown and a backoff timer firing. On thread A + // we might call shutdown which removes the backoff timer. On thread B the backoff + // timer might fire at the same time and be blocked by the state lock. In this case + // we would look for the backoff timer that was removed just before by the shutdown. + return .none } - // In `nextActionForFailedConnection` a decision will be made whether the failed - // connection should be replaced or removed. - return self.nextActionForFailedConnection(at: index, context: context) } mutating func connectionIdleTimeout(_ connectionID: Connection.ID) -> Action { diff --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests+XCTest.swift index 590f078ab..e892f957f 100644 --- a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests+XCTest.swift @@ -37,6 +37,7 @@ extension HTTPConnectionPool_HTTP1StateMachineTests { ("testParkedConnectionTimesOut", testParkedConnectionTimesOut), ("testConnectionPoolFullOfParkedConnectionsIsShutdownImmediately", testConnectionPoolFullOfParkedConnectionsIsShutdownImmediately), ("testParkedConnectionTimesOutButIsAlsoClosedByRemote", testParkedConnectionTimesOutButIsAlsoClosedByRemote), + ("testConnectionBackoffVsShutdownRace", testConnectionBackoffVsShutdownRace), ] } } diff --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests.swift b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests.swift index f781349b9..28059063a 100644 --- a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1StateTests.swift @@ -587,4 +587,43 @@ class HTTPConnectionPool_HTTP1StateMachineTests: XCTestCase { // triggered by timer XCTAssertEqual(state.connectionIdleTimeout(connection.id), .none) } + + func testConnectionBackoffVsShutdownRace() { + let elg = EmbeddedEventLoopGroup(loops: 2) + defer { XCTAssertNoThrow(try elg.syncShutdownGracefully()) } + + var state = HTTPConnectionPool.StateMachine( + eventLoopGroup: elg, + idGenerator: .init(), + maximumConcurrentHTTP1Connections: 6 + ) + + let mockRequest = MockHTTPRequest(eventLoop: elg.next(), requiresEventLoopForChannel: false) + let request = HTTPConnectionPool.Request(mockRequest) + + let executeAction = state.executeRequest(request) + guard case .createConnection(let connectionID, on: let connEL) = executeAction.connection else { + return XCTFail("Expected to create a connection") + } + + XCTAssertEqual(executeAction.request, .scheduleRequestTimeout(for: request, on: mockRequest.eventLoop)) + + let failAction = state.failedToCreateNewConnection(HTTPClientError.cancelled, connectionID: connectionID) + guard case .scheduleBackoffTimer(connectionID, backoff: _, on: let timerEL) = failAction.connection else { + return XCTFail("Expected to create a backoff timer") + } + XCTAssert(timerEL === connEL) + XCTAssertEqual(failAction.request, .none) + + let shutdownAction = state.shutdown() + guard case .cleanupConnections(let context, isShutdown: .yes(unclean: true)) = shutdownAction.connection else { + return XCTFail("Expected to cleanup") + } + XCTAssertEqual(context.close.count, 0) + XCTAssertEqual(context.cancel.count, 0) + XCTAssertEqual(context.connectBackoff, [connectionID]) + XCTAssertEqual(shutdownAction.request, .failRequestsAndCancelTimeouts([request], HTTPClientError.cancelled)) + + XCTAssertEqual(state.connectionCreationBackoffDone(connectionID), .none) + } }