Skip to content

Commit 91f43d6

Browse files
sushanbclaude
andauthored
xds/resolver: drop field references on Close to prevent retention across ClientConn recycle (#9301)
Retention is via multiple field references off `*xdsResolver` that survive `Close`: - `r.cc` → the `ClientConn`, whose `ServiceConfig` still transitively references the resolver - `r.curConfigSelector` → captured by the `ClientConn`'s `ServiceConfig` via `UpdateState` - `r.dm` / `r.xdsClient` → watcher metadata held by the shared `xdsclient.DefaultPool` singleton, which survives every ClientConn lifetime Individually each field is a benign back-reference; together they form multiple independent retention chains through externally-anchored state. Go's tracing GC handles cycles fine, but these aren't cycles — they're chains rooted at package-global state (the shared xdsClient pool) and other ClientConns that stay alive. Explicitly dropping the field references at the end of `Close` leaves the resolver struct with no outbound edges. The entire per-channel resolver + CDS balancer subtree (roughly 40 `CallbackSerializer`s, JSON-parsed service configs, `attributes.Attributes` chains, cloned `[]resolver.Address` slices per channel) becomes GC-collectible. RELEASE NOTES: None --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6d697e4 commit 91f43d6

3 files changed

Lines changed: 34 additions & 0 deletions

File tree

clientconn.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,6 +1212,12 @@ func (cc *ClientConn) Close() error {
12121212
cc.mu.Unlock()
12131213

12141214
cc.resolverWrapper.close()
1215+
// Swap in a default ConfigSelector so the resolver-provided one (which may
1216+
// transitively retain resolver-owned state such as parsed service configs
1217+
// and cluster maps) is released once in-flight RPCs finish selecting on
1218+
// it. Done after the resolver has closed so no further UpdateState calls
1219+
// race with this swap.
1220+
cc.safeConfigSelector.UpdateConfigSelector(&defaultConfigSelector{nil})
12151221
// The order of closing matters here since the balancer wrapper assumes the
12161222
// picker is closed before it is closed.
12171223
cc.pickerWrapper.close()

internal/xds/resolver/xds_resolver.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,21 @@ func (r *xdsResolver) Close() {
302302
if r.xdsClientClose != nil {
303303
r.xdsClientClose()
304304
}
305+
// Drop field references so the resolver isn't retained across ClientConn
306+
// close via any cross-object retention chain (e.g. ClientConn holding the
307+
// ServiceConfig that captures the configSelector, shared xDS-client
308+
// watcher metadata, channelz trace entries). Each nil individually looks
309+
// harmless; the combination breaks the multi-path retention observed when
310+
// callers repeatedly Close and redial DirectPath channels.
311+
r.cc = nil
312+
r.curConfigSelector = nil
313+
r.dm = nil
314+
r.xdsClient = nil
315+
r.xdsClientClose = nil
316+
r.httpFilters = nil
317+
r.activeClusters = nil
318+
r.activePlugins = nil
319+
r.xdsConfig = nil
305320
r.logger.Infof("Shutdown")
306321
}
307322

internal/xds/xdsdepmgr/xds_dependency_manager.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,12 @@ func (m *DependencyManager) Close() {
203203
dnsResolver.stop()
204204
delete(m.dnsResolvers, name)
205205
}
206+
207+
// Drop references to externally-owned collaborators so the dependency
208+
// manager itself doesn't retain them once Close returns. All in-flight
209+
// callbacks re-check m.stopped under m.mu before touching these fields.
210+
m.watcher = nil
211+
m.xdsClient = nil
206212
}
207213

208214
// annotateErrorWithNodeID annotates the given error with the provided xDS node
@@ -999,6 +1005,13 @@ func (m *DependencyManager) SubscribeToCluster(name string) func() {
9991005
func (m *DependencyManager) unsubscribeFromCluster(name string) {
10001006
m.mu.Lock()
10011007
defer m.mu.Unlock()
1008+
// Balancers can invoke the unsubscribe closure returned by
1009+
// SubscribeToCluster after Close has already torn down all watchers and
1010+
// dropped m.watcher / m.xdsClient. Match the m.stopped guard used by
1011+
// every other post-Close callback in this file.
1012+
if m.stopped {
1013+
return
1014+
}
10021015
c := m.clusterSubscriptions[name]
10031016
c.dynamicRefCount--
10041017
// This should not happen as unsubscribe returned from the

0 commit comments

Comments
 (0)