Skip to content

core: Fix pick_first NPE when accepting resolved addresses and in CONNECTING state#12814

Open
schiemon wants to merge 1 commit into
grpc:masterfrom
schiemon:fix/pick-first-leaf-npe-12796
Open

core: Fix pick_first NPE when accepting resolved addresses and in CONNECTING state#12814
schiemon wants to merge 1 commit into
grpc:masterfrom
schiemon:fix/pick-first-leaf-npe-12796

Conversation

@schiemon
Copy link
Copy Markdown

@schiemon schiemon commented May 19, 2026

This PR resolves #12796.

It makes sure that whenever PickFirstLeafLoadBalancer transitions into CONNECTING the current address in the addressIndex has a corresponding subchannel. This prevents an NPE when hitting

if (rawConnectivityState == READY
|| (rawConnectivityState == CONNECTING
&& (!enableHappyEyeballs || addressIndex.isValid()))) {
// If the previous ready (or connecting) subchannel exists in new address list,
// keep this connection and don't create new subchannels. Happy Eyeballs is excluded when
// connecting, because it allows multiple attempts simultaneously, thus is fine to start at
// the beginning.
SocketAddress previousAddress = addressIndex.getCurrentAddress();
addressIndex.updateGroups(newImmutableAddressGroups);
if (addressIndex.seekTo(previousAddress)) {
SubchannelData subchannelData = subchannels.get(previousAddress);
subchannelData.getSubchannel().updateAddresses(addressIndex.getCurrentEagAsList());

in acceptResolvedAddresses in some situations.

Note that for READY, a state where this path can also be entered, this is already guaranteed.

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 19, 2026

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: schiemon / name: szymon.habrainski (c76956a)

// addresses in the updated list.
subchannelData.updateState(newState);
if (rawConnectivityState == TRANSIENT_FAILURE || concludedState == TRANSIENT_FAILURE) {
if (newState == CONNECTING) {
Copy link
Copy Markdown
Author

@schiemon schiemon May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If concludedState == TRANSIENT_FAILURE but rawConnectivityState != TRANSIENT_FAILURE, shouldn't we adapt our rawConnectivityState?


if (rawConnectivityState == READY
|| (rawConnectivityState == CONNECTING
&& (!enableHappyEyeballs || addressIndex.isValid()))) {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not

Suggested change
&& addressIndex.isValid())) {

? If we !enableHappyEyeballs and !addressIndex.isValid() then we get an exception when calling addressIndex.getCurrentAddress().

addressIndex.updateGroups(newImmutableAddressGroups);
if (addressIndex.seekTo(previousAddress)) {
SubchannelData subchannelData = subchannels.get(previousAddress);
subchannelData.getSubchannel().updateAddresses(addressIndex.getCurrentEagAsList());
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With HE we have potentionally multiple connecting subchannels: why aren't we updating them all?

@schiemon schiemon marked this pull request as ready for review May 19, 2026 14:03
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.

NPE in PickFirstLeafLoadBalancer

1 participant