From 6ef96baf70d76042ec76d08c507a087ce51f57d5 Mon Sep 17 00:00:00 2001 From: brennobemoura <37243584+brennobemoura@users.noreply.github.com> Date: Thu, 27 Aug 2026 08:16:00 -0300 Subject: [PATCH] Fix crash when a connection release races with its own close/fail http1ConnectionReleased force-unwrapped releaseConnection(_:), crashing via preconditionFailure if the connectionID was no longer in the state machine's connections array. Its sibling http1ConnectionClosed already handled this gracefully (guard let ... else { return .none }), because the same race exists for both events: the channel's closeFuture and taskCompleted() callbacks run on the same EventLoop but their relative order depends on I/O timing, so a connection can be torn down (idle timeout, peer closing early, HTTP1->HTTP2 migration) right before its queued release event is processed. Found via the new Apple platform CI: enabling apple-tests exposed this on iOS/iPadOS/watchOS simulators (crashing consistently around testBiDirectionalStreamingEarly200 with "Fatal error: A connection that we don't know was released? Something is very wrong..."), while macOS/tvOS/visionOS/Catalyst didn't hit the race in the time it takes to run the suite. Upstream's own CI never ran this suite against an Apple platform at all, so the bug had no way of surfacing there. Same fix applied at both call sites (HTTP1StateMachine and HTTP2StateMachine), mirroring the existing http1ConnectionClosed pattern in each. Co-Authored-By: Claude Sonnet 5 --- .../HTTPConnectionPool+HTTP1Connections.swift | 12 +++++++++--- .../HTTPConnectionPool+HTTP1StateMachine.swift | 7 ++++++- .../HTTPConnectionPool+HTTP2StateMachine.swift | 9 +++++++-- .../HTTPConnectionPool+HTTP1ConnectionsTest.swift | 4 +++- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift index 3cdf51869..63c0429f6 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1Connections.swift @@ -496,10 +496,16 @@ extension HTTPConnectionPool { /// - Parameter connectionID: The released connection's id. /// - Returns: An index and an IdleConnectionContext to determine the next action for the now idle connection. /// Call ``leaseConnection(at:)`` or ``closeConnection(at:)`` with the supplied index after - /// this. If you want to park the connection no further call is required. - mutating func releaseConnection(_ connectionID: Connection.ID) -> (Int, IdleConnectionContext) { + /// this. If you want to park the connection no further call is required. `nil` if the + /// connection is no longer known to the state machine. + mutating func releaseConnection(_ connectionID: Connection.ID) -> (Int, IdleConnectionContext)? { guard let index = self.connections.firstIndex(where: { $0.connectionID == connectionID }) else { - preconditionFailure("A connection that we don't know was released? Something is very wrong...") + // The connection's close/failure may have already been processed by the state + // machine (e.g. the peer closed the connection right as the response finished, + // racing with this release notification) — that path already removed the + // connection and found new work for any queued requests, so there's nothing left + // to do for this now-stale release. + return nil } self.connections[index].release() diff --git a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift index 395064377..7ecb3a9d8 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP1StateMachine.swift @@ -348,7 +348,12 @@ extension HTTPConnectionPool { } mutating func http1ConnectionReleased(_ connectionID: Connection.ID) -> Action { - let (index, context) = self.connections.releaseConnection(connectionID) + guard let (index, context) = self.connections.releaseConnection(connectionID) else { + // The connection was already closed/failed by the time this release was + // processed; that path already handled cleanup. See `http1ConnectionClosed` + // above for the same pattern. + return .none + } return .init(self.nextActionForIdleConnection(at: index, context: context)) } diff --git a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP2StateMachine.swift b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP2StateMachine.swift index 67a07e6dd..4e9d7a064 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP2StateMachine.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/State Machine/HTTPConnectionPool+HTTP2StateMachine.swift @@ -563,9 +563,14 @@ extension HTTPConnectionPool { } mutating func http1ConnectionReleased(_ connectionID: Connection.ID) -> Action { - // It is save to bang the http1Connections here. If we get this callback but we don't have + // It is safe to bang the http1Connections here. If we get this callback but we don't have // http1 connections something has gone terribly wrong. - let (index, _) = self.http1Connections!.releaseConnection(connectionID) + guard let (index, _) = self.http1Connections!.releaseConnection(connectionID) else { + // The connection was already closed/failed by the time this release was + // processed; that path already handled cleanup. See `http1ConnectionClosed` + // above for the same pattern. + return .none + } // Any http1 connection that becomes idle should be closed right away after the transition // to http2. let connection = self.http1Connections!.closeConnection(at: index) diff --git a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift index 89f3bf7b5..01982fcd1 100644 --- a/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPConnectionPool+HTTP1ConnectionsTest.swift @@ -380,7 +380,9 @@ class HTTPConnectionPool_HTTP1ConnectionsTests: XCTestCase { XCTAssertEqual(connections.stats.connecting, 1) XCTAssertFalse(connections.isEmpty) - let (releaseIndex, _) = connections.releaseConnection(lease.id) + guard let (releaseIndex, _) = connections.releaseConnection(lease.id) else { + return XCTFail("Expected that the connection is remembered") + } XCTAssertEqual(connections.closeConnection(at: releaseIndex), lease) XCTAssertFalse(connections.isEmpty)