diff --git a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift index 701e630c2..5859e619a 100644 --- a/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift +++ b/Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift @@ -111,7 +111,7 @@ final class HTTP2Connection { deinit { guard case .closed = self.state else { - preconditionFailure("Connection must be closed, before we can deinit it") + preconditionFailure("Connection must be closed, before we can deinit it. Current state: \(self.state)") } } @@ -129,7 +129,7 @@ final class HTTP2Connection { delegate: delegate, logger: logger ) - return connection.start().map { maxStreams in (connection, maxStreams) } + return connection._start0().map { maxStreams in (connection, maxStreams) } } func executeRequest(_ request: HTTPExecutableRequest) { @@ -164,15 +164,23 @@ final class HTTP2Connection { return promise.futureResult } - private func start() -> EventLoopFuture { + func _start0() -> EventLoopFuture { self.channel.eventLoop.assertInEventLoop() let readyToAcceptConnectionsPromise = self.channel.eventLoop.makePromise(of: Int.self) self.state = .starting(readyToAcceptConnectionsPromise) self.channel.closeFuture.whenComplete { _ in - self.state = .closed - self.delegate.http2ConnectionClosed(self) + switch self.state { + case .initialized, .closed: + preconditionFailure("invalid state \(self.state)") + case .starting(let readyToAcceptConnectionsPromise): + self.state = .closed + readyToAcceptConnectionsPromise.fail(HTTPClientError.remoteConnectionClosed) + case .active, .closing: + self.state = .closed + self.delegate.http2ConnectionClosed(self) + } } do { @@ -258,16 +266,26 @@ final class HTTP2Connection { private func shutdown0() { self.channel.eventLoop.assertInEventLoop() - self.state = .closing + switch self.state { + case .active: + self.state = .closing - // inform all open streams, that the currently running request should be cancelled. - self.openStreams.forEach { box in - box.channel.triggerUserOutboundEvent(HTTPConnectionEvent.shutdownRequested, promise: nil) - } + // inform all open streams, that the currently running request should be cancelled. + self.openStreams.forEach { box in + box.channel.triggerUserOutboundEvent(HTTPConnectionEvent.shutdownRequested, promise: nil) + } - // inform the idle connection handler, that connection should be closed, once all streams - // are closed. - self.channel.triggerUserOutboundEvent(HTTPConnectionEvent.shutdownRequested, promise: nil) + // inform the idle connection handler, that connection should be closed, once all streams + // are closed. + self.channel.triggerUserOutboundEvent(HTTPConnectionEvent.shutdownRequested, promise: nil) + + case .closed, .closing: + // we are already closing/closed and we need to tolerate this + break + + case .initialized, .starting: + preconditionFailure("invalid state \(self.state)") + } } } diff --git a/Tests/AsyncHTTPClientTests/HTTP2ConnectionTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTP2ConnectionTests+XCTest.swift index 9f9582d9f..06b60f757 100644 --- a/Tests/AsyncHTTPClientTests/HTTP2ConnectionTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTP2ConnectionTests+XCTest.swift @@ -26,6 +26,7 @@ extension HTTP2ConnectionTests { static var allTests: [(String, (HTTP2ConnectionTests) -> () throws -> Void)] { return [ ("testCreateNewConnectionFailureClosedIO", testCreateNewConnectionFailureClosedIO), + ("testConnectionToleratesShutdownEventsAfterAlreadyClosed", testConnectionToleratesShutdownEventsAfterAlreadyClosed), ("testSimpleGetRequest", testSimpleGetRequest), ("testEveryDoneRequestLeadsToAStreamAvailableCall", testEveryDoneRequestLeadsToAStreamAvailableCall), ("testCancelAllRunningRequests", testCancelAllRunningRequests), diff --git a/Tests/AsyncHTTPClientTests/HTTP2ConnectionTests.swift b/Tests/AsyncHTTPClientTests/HTTP2ConnectionTests.swift index a9ff14f49..652884a84 100644 --- a/Tests/AsyncHTTPClientTests/HTTP2ConnectionTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTP2ConnectionTests.swift @@ -42,6 +42,30 @@ class HTTP2ConnectionTests: XCTestCase { ).wait()) } + func testConnectionToleratesShutdownEventsAfterAlreadyClosed() { + let embedded = EmbeddedChannel() + XCTAssertNoThrow(try embedded.connect(to: SocketAddress(ipAddress: "127.0.0.1", port: 3000)).wait()) + + let logger = Logger(label: "test.http2.connection") + let connection = HTTP2Connection( + channel: embedded, + connectionID: 0, + decompression: .disabled, + delegate: TestHTTP2ConnectionDelegate(), + logger: logger + ) + let startFuture = connection._start0() + + XCTAssertNoThrow(try embedded.close().wait()) + // to really destroy the channel we need to tick once + embedded.embeddedEventLoop.run() + + XCTAssertThrowsError(try startFuture.wait()) + + // should not crash + connection.shutdown() + } + func testSimpleGetRequest() { let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) let eventLoop = eventLoopGroup.next()