Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultiChildLoadBalancer with EquivalentAddressGroup with multiple addresses throws NPE #10768

Closed
thegridman opened this issue Dec 16, 2023 · 1 comment · Fixed by #10769
Closed
Assignees
Milestone

Comments

@thegridman
Copy link

gRPC Java version 1.60.0

What is your environment?

This fails on all our environments, Linux, MacOS etc

What did you expect to see?

We expect the load balancer to work with a single EquivalentAddressGroup containing multiple addresses.

What did you see instead?

In our client code we receive a Runtime io.grpc.StatusRuntimeException: INTERNAL: Panic! This is a bug!

After debugging the client we tracked it down to this code in the MultiChildLoadBalancer.Endpoint constructor

    public Endpoint(EquivalentAddressGroup eag) {
      checkNotNull(eag, "eag");

      addrs = new String[eag.getAddresses().size()];
      int i = 0;
      for (SocketAddress address : eag.getAddresses()) {
        addrs[i] = address.toString();
      }
      Arrays.sort(addrs);

      hashCode = Arrays.hashCode(addrs);
    }

You can see that the code iterates over the addresses in the eag and adds the toString() of each to the string array. But it does not increment the index i so all addresses go to the first array element, the rest are null. This throws an NPE when Arrays.sort() is called.

I assume it should be a trivial fix like addrs[i++] = address.toString();

@thegridman
Copy link
Author

thegridman commented Dec 16, 2023

I am able to work around this by changing my custom NameResolver.

Currently I do this

List<SocketAddress> list = // my list of server socket addresses

NameResolver.ResolutionResult result = NameResolver.ResolutionResult.newBuilder()
                                    .setAddresses(Collections.singletonList(new EquivalentAddressGroup(list, attrs)))
                                    .setAttributes(Attributes.EMPTY)
                                    .build();

// result is then passed to a NameResolver.Listener2
listener.onResult(result);

If, instead of creating a EquivalentAddressGroup with multiple addresses I create multiple EquivalentAddressGroup instances with a single address, then it works (as there is only one address passed to the broken code above).

List<SocketAddress> list = // my list of server socket addresses

List<EquivalentAddressGroup> groupList = list.stream()
        .map(addr -> new EquivalentAddressGroup(addr, attrs))
        .collect(Collectors.toList());

NameResolver.ResolutionResult result = NameResolver.ResolutionResult.newBuilder()
                                    .setAddresses(groupList)
                                    .setAttributes(Attributes.EMPTY)
                                    .build();

// result is then passed to a NameResolver.Listener2
listener.onResult(result);

I assume this work around is still valid, one EquivalentAddressGroup with multiple addresses or multiple EquivalentAddressGroup with one address will not cause me a problem somewhere else.

@YifeiZhuang YifeiZhuang added the TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved label Dec 18, 2023
@YifeiZhuang YifeiZhuang added this to the 1.61 milestone Dec 18, 2023
larry-safran added a commit to larry-safran/grpc-java that referenced this issue Dec 18, 2023
larry-safran added a commit to larry-safran/grpc-java that referenced this issue Dec 19, 2023
@YifeiZhuang YifeiZhuang removed the TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved label Feb 2, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants