Skip to content

Commit 8f57f68

Browse files
authored
Shutdown quiesced conn managers when removed from pool (#2268)
Motivation: In some rare situations the connection manager can leak a promise on deinit. The connection pool maintains the refs to these managers and they are only dropped in two situations: 1. when the pool is shutdown 2. when a quiescing connection is removed from the pool When the pool is shutdown all conn managers are also shutdown (which will complete all promises stored in its state). When a conn is going away the conn manager relies on events from the channel pipeline to inform it that it can close. When it closes the pool will remove the ref to the conn manager and do a few bits of cleanup. If the conn closes gracefully then all is well. However, if that isn't the case then a reconnect may kick in (creating new promises). At this point there is a conn manager attempting reconnects but which is no longer managed by the pool: this is a bug in and of itself. The conn manager is also kept alive by a ref from the channel it is currently managing (via a channel handler), it's possible that at some point during the connection lifecycle that that ref is dropped and there's nothing keeping the conn manager alive, but this is speculative. The fix for this is straightforward: for conn managers which are quiescing shut them down when they are removed from the pool. Modifications: - Shutdown conn manager when its last connection is dropped Result: Fewer leaking promises
1 parent a531ab9 commit 8f57f68

File tree

2 files changed

+6
-0
lines changed

2 files changed

+6
-0
lines changed

Sources/GRPC/ConnectionManager.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,6 +1218,11 @@ extension ConnectionManager {
12181218
internal func startConnecting() {
12191219
self.manager.startConnecting()
12201220
}
1221+
1222+
/// Shutdown the manager now.
1223+
internal func shutdownNow() {
1224+
self.manager._shutdown(mode: .forceful, promise: self.manager.eventLoop.makePromise())
1225+
}
12211226
}
12221227
}
12231228

Sources/GRPC/ConnectionPool/ConnectionPool.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -760,6 +760,7 @@ extension ConnectionPool: ConnectionManagerConnectivityDelegate {
760760
assert(hadActiveConnection)
761761
if let removed = self._connections.removeValue(forKey: manager.id) {
762762
removed.manager.sync.http2Delegate = nil
763+
removed.manager.sync.shutdownNow() // Manager may have internal state to tear down.
763764
self.delegate?.connectionClosed(id: .init(removed.manager.id), error: nil)
764765
self.delegate?.connectionRemoved(id: .init(removed.manager.id))
765766
}

0 commit comments

Comments
 (0)