Skip to content

Commit 3b3b5d0

Browse files
committed
Tolerate shutdown message after channel is closed
### Motivation A channel can close unexpectedly if something goes wrong. We may in the meantime have scheduled the connection for graceful shutdown but the connection has not yet seen the message. We need to still accept the shutdown message and just ignore it if we are already closed. ### Modification - ignore calls to shutdown if the channel is already closed - add a test which would previously crash because we have transition from the closed state to the closing state and we hit the deinit precondition - include the current state in preconditions if we are in the wrong state ### Result We don’t hit the precondition in the deinit in the scenario described above and have more descriptive crashes if something still goes wrong.
1 parent 8b84142 commit 3b3b5d0

File tree

2 files changed

+51
-12
lines changed

2 files changed

+51
-12
lines changed

Sources/AsyncHTTPClient/ConnectionPool/HTTP2/HTTP2Connection.swift

+29-12
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ final class HTTP2Connection {
111111

112112
deinit {
113113
guard case .closed = self.state else {
114-
preconditionFailure("Connection must be closed, before we can deinit it")
114+
preconditionFailure("Connection must be closed, before we can deinit it. Current state: \(self.state)")
115115
}
116116
}
117117

@@ -164,15 +164,23 @@ final class HTTP2Connection {
164164
return promise.futureResult
165165
}
166166

167-
private func start() -> EventLoopFuture<Int> {
167+
func start() -> EventLoopFuture<Int> {
168168
self.channel.eventLoop.assertInEventLoop()
169169

170170
let readyToAcceptConnectionsPromise = self.channel.eventLoop.makePromise(of: Int.self)
171171

172172
self.state = .starting(readyToAcceptConnectionsPromise)
173173
self.channel.closeFuture.whenComplete { _ in
174-
self.state = .closed
175-
self.delegate.http2ConnectionClosed(self)
174+
switch self.state {
175+
case .initialized, .closed:
176+
preconditionFailure("invalid state \(self.state)")
177+
case .starting(let readyToAcceptConnectionsPromise):
178+
self.state = .closed
179+
readyToAcceptConnectionsPromise.fail(HTTPClientError.remoteConnectionClosed)
180+
case .active, .closing:
181+
self.state = .closed
182+
self.delegate.http2ConnectionClosed(self)
183+
}
176184
}
177185

178186
do {
@@ -258,16 +266,25 @@ final class HTTP2Connection {
258266
private func shutdown0() {
259267
self.channel.eventLoop.assertInEventLoop()
260268

261-
self.state = .closing
269+
switch self.state {
270+
case .active, .starting:
271+
self.state = .closing
262272

263-
// inform all open streams, that the currently running request should be cancelled.
264-
self.openStreams.forEach { box in
265-
box.channel.triggerUserOutboundEvent(HTTPConnectionEvent.shutdownRequested, promise: nil)
266-
}
273+
// inform all open streams, that the currently running request should be cancelled.
274+
self.openStreams.forEach { box in
275+
box.channel.triggerUserOutboundEvent(HTTPConnectionEvent.shutdownRequested, promise: nil)
276+
}
267277

268-
// inform the idle connection handler, that connection should be closed, once all streams
269-
// are closed.
270-
self.channel.triggerUserOutboundEvent(HTTPConnectionEvent.shutdownRequested, promise: nil)
278+
// inform the idle connection handler, that connection should be closed, once all streams
279+
// are closed.
280+
self.channel.triggerUserOutboundEvent(HTTPConnectionEvent.shutdownRequested, promise: nil)
281+
282+
case .closed:
283+
// we are already closed and we need to tolerate this
284+
break
285+
case .initialized, .closing:
286+
preconditionFailure("invalid state \(self.state)")
287+
}
271288
}
272289
}
273290

Tests/AsyncHTTPClientTests/HTTP2ConnectionTests.swift

+22
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,28 @@ class HTTP2ConnectionTests: XCTestCase {
4141
logger: logger
4242
).wait())
4343
}
44+
45+
func testConnectionToleratesShutdownEventsAfterAlreadyClosed() {
46+
let embedded = EmbeddedChannel()
47+
XCTAssertNoThrow(try embedded.connect(to: SocketAddress(ipAddress: "127.0.0.1", port: 3000)).wait())
48+
49+
let logger = Logger(label: "test.http2.connection")
50+
let connection = HTTP2Connection(
51+
channel: embedded,
52+
connectionID: 0,
53+
decompression: .disabled,
54+
delegate: TestHTTP2ConnectionDelegate(),
55+
logger: logger
56+
)
57+
_ = connection.start()
58+
59+
XCTAssertNoThrow(try embedded.close().wait())
60+
// to really destroy the channel we need to tick once
61+
embedded.embeddedEventLoop.run()
62+
63+
// should not crash
64+
connection.shutdown()
65+
}
4466

4567
func testSimpleGetRequest() {
4668
let eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1)

0 commit comments

Comments
 (0)