From 70942c7f5854eadf9d33f7a6d31713584bf8a864 Mon Sep 17 00:00:00 2001 From: George Barnett Date: Mon, 1 Dec 2025 16:25:19 +0000 Subject: [PATCH] Shutdown quiesced conn managers when removed from pool 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 --- Sources/GRPC/ConnectionManager.swift | 5 +++++ Sources/GRPC/ConnectionPool/ConnectionPool.swift | 1 + 2 files changed, 6 insertions(+) diff --git a/Sources/GRPC/ConnectionManager.swift b/Sources/GRPC/ConnectionManager.swift index 93cbc7527..af483adb7 100644 --- a/Sources/GRPC/ConnectionManager.swift +++ b/Sources/GRPC/ConnectionManager.swift @@ -1218,6 +1218,11 @@ extension ConnectionManager { internal func startConnecting() { self.manager.startConnecting() } + + /// Shutdown the manager now. + internal func shutdownNow() { + self.manager._shutdown(mode: .forceful, promise: self.manager.eventLoop.makePromise()) + } } } diff --git a/Sources/GRPC/ConnectionPool/ConnectionPool.swift b/Sources/GRPC/ConnectionPool/ConnectionPool.swift index 7ae82b6e6..c8d5dd149 100644 --- a/Sources/GRPC/ConnectionPool/ConnectionPool.swift +++ b/Sources/GRPC/ConnectionPool/ConnectionPool.swift @@ -760,6 +760,7 @@ extension ConnectionPool: ConnectionManagerConnectivityDelegate { assert(hadActiveConnection) if let removed = self._connections.removeValue(forKey: manager.id) { removed.manager.sync.http2Delegate = nil + removed.manager.sync.shutdownNow() // Manager may have internal state to tear down. self.delegate?.connectionClosed(id: .init(removed.manager.id), error: nil) self.delegate?.connectionRemoved(id: .init(removed.manager.id)) }