xds: make ControlPlaneClient.shutdown() synchronous on syncContext by ran-su · Pull Request #13136 · grpc/grpc-java · GitHub
Skip to content

xds: make ControlPlaneClient.shutdown() synchronous on syncContext - #13136

Open
ran-su wants to merge 1 commit into
grpc:masterfrom
ran-su:fix-xds-fallback-shutdown-npe
Open

ran-su wants to merge 1 commit into
grpc:masterfrom
ran-su:fix-xds-fallback-shutdown-npe

Conversation

@ran-su

@ran-su ran-su commented Oct 20, 2026

Copy link
Copy Markdown
Member

When a higher-priority xDS server responds while a lower-priority fallback ControlPlaneClient already has a DiscoveryResponse runnable queued in the shared SynchronizationContext, calling shutdownLowerPriorityCpcs() removes the fallback entry from serverLrsClientMap and invokes cpc.shutdown().

Because ControlPlaneClient.shutdown() previously scheduled its cleanup via syncContext.execute(...), the cleanup was placed at the tail of the queue behind the already-queued response runnable. When that queued response runnable executed, adsStream had not been closed yet, so handleResourceUpdate() looked up serverLrsClientMap.get(serverInfo) (now null) and threw a NullPointerException when calling controlPlaneClient.ackResponse(...).

Fix this by:

  1. Executing ControlPlaneClient.shutdown() directly on syncContext (enforcing syncContext.throwIfNotInThisSynchronizationContext()), which all callers already satisfy, so adsStream is closed immediately before any subsequently queued runnables run.
  2. Adding a defensive null check for controlPlaneClient in XdsClientImpl.handleResourceUpdate().

more context: http://b/469140342

@ran-su
ran-su force-pushed the fix-xds-fallback-shutdown-npe branch from fdcd2cf to 15eb515 Compare October 20, 2026 18:31
When a higher-priority xDS server responds while a lower-priority fallback ControlPlaneClient already has a DiscoveryResponse runnable queued in the shared SynchronizationContext, calling shutdownLowerPriorityCpcs() removes the fallback entry from serverLrsClientMap and invokes cpc.shutdown().

Because ControlPlaneClient.shutdown() previously scheduled its cleanup via syncContext.execute(...), the cleanup was placed at the tail of the queue behind the already-queued response runnable. When that queued response runnable executed, adsStream had not been closed yet, so handleResourceUpdate() looked up serverLrsClientMap.get(serverInfo) (now null) and threw a NullPointerException when calling controlPlaneClient.ackResponse(...).

Fix this by:
1. Executing ControlPlaneClient.shutdown() directly on syncContext (enforcing syncContext.throwIfNotInThisSynchronizationContext()), which all callers already satisfy, so adsStream is closed immediately before any subsequently queued runnables run.
2. Adding a defensive null check for controlPlaneClient in XdsClientImpl.handleResourceUpdate().
@ran-su
ran-su force-pushed the fix-xds-fallback-shutdown-npe branch from 15eb515 to 092d3fc Compare October 20, 2026 18:37
@ran-su
ran-su requested a review from shivaspeaks October 21, 2026 18:18

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant