#19916

Committed 18 Jul 2025 04:05PM UTC coverage: 88.586% (+0.006%) from 88.58%

Build # #19916

Build Type

push

github

Committed by

ejona86

Commit Message

LBs should avoid calling LBs after lb.shutdown()

LoadBalancers shouldn't be called after shutdown(), but RingHashLb could
have enqueued work to the SynchronizationContext that executed after
shutdown(). This commit fixes problems discovered when auditing all LBs
usage of the syncContext for that type of problem.

Similarly, PickFirstLb could have requested a new connection after
shutdown(). We want to avoid that sort of thing too.

RingHashLb's test changed from CONNECTING to TRANSIENT_FAILURE to get
the latest picker. Because two subchannels have failed it will be in
TRANSIENT_FAILURE. Previously the test was using an older picker with
out-of-date subchannelView, and the verifyConnection() was too imprecise
to notice it was creating the wrong subchannel.

As discovered in b/430347751, where ClusterImplLb was seeing a new
subchannel being called after the child LB was shutdown (the shutdown
itself had been caused by RingHashConfig not implementing equals() and
was fixed by a8de9f07ab, which caused ClusterResolverLb to replace its
state):

```
java.lang.NullPointerException
	at io.grpc.xds.ClusterImplLoadBalancer$ClusterImplLbHelper.createClusterLocalityFromAttributes(ClusterImplLoadBalancer.java:322)
	at io.grpc.xds.ClusterImplLoadBalancer$ClusterImplLbHelper.createSubchannel(ClusterImplLoadBalancer.java:236)
	at io.grpc.util.ForwardingLoadBalancerHelper.createSubchannel(ForwardingLoadBalancerHelper.java:47)
	at io.grpc.util.ForwardingLoadBalancerHelper.createSubchannel(ForwardingLoadBalancerHelper.java:47)
	at io.grpc.internal.PickFirstLeafLoadBalancer.createNewSubchannel(PickFirstLeafLoadBalancer.java:527)
	at io.grpc.internal.PickFirstLeafLoadBalancer.requestConnection(PickFirstLeafLoadBalancer.java:459)
	at io.grpc.internal.PickFirstLeafLoadBalancer.acceptResolvedAddresses(PickFirstLeafLoadBalancer.java:174)
	at io.grpc.xds.LazyLoadBalancer$LazyDelegate.activate(LazyLoadBalancer.java:64)
	at io.grpc.xds.LazyLoadBalancer$LazyDelegate.requestConnec... (continued)

Run Details

34653 of 39118 relevant lines covered (88.59%)

0.89 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

81.4

/../xds/src/main/java/io/grpc/xds/LazyLoadBalancer.java

/*
 * Copyright 2024 The gRPC Authors
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package io.grpc.xds;

import com.google.common.base.Preconditions;
import io.grpc.ConnectivityState;
import io.grpc.LoadBalancer;
import io.grpc.Status;
import io.grpc.util.ForwardingLoadBalancer;

/**
 * A load balancer that starts in IDLE instead of CONNECTING. Once it starts connecting, it
 * instantiates its delegate.
 */
final class LazyLoadBalancer extends ForwardingLoadBalancer {
  private LoadBalancer delegate;

  public LazyLoadBalancer(Helper helper, LoadBalancer.Factory delegateFactory) {
    this.delegate = new LazyDelegate(helper, delegateFactory);
  }

  @Override
  protected LoadBalancer delegate() {
    return delegate;
  }

  @Override
  public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
    return delegate.acceptResolvedAddresses(resolvedAddresses);
  }

  private final class LazyDelegate extends LoadBalancer {
    private final Helper helper;
    private final LoadBalancer.Factory delegateFactory;
    private ResolvedAddresses addresses;
    private Status error;
    private boolean updatedBalancingState;

    public LazyDelegate(Helper helper, LoadBalancer.Factory delegateFactory) {
      this.helper = Preconditions.checkNotNull(helper, "helper");
      this.delegateFactory = Preconditions.checkNotNull(delegateFactory, "delegateFactory");
    }

    private LoadBalancer activate() {
      if (delegate != this) {
        return delegate;
      }
      delegate = delegateFactory.newLoadBalancer(helper);
      if (addresses != null) {
        delegate.acceptResolvedAddresses(addresses);
      }
      if (error != null) {
        delegate.handleNameResolutionError(error);
      }
      return delegate;
    }

    @Override
    public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
      this.addresses = resolvedAddresses;
      this.error = null;
      initializeBalancingState();
      return Status.OK;
    }

    @Override
    public void handleNameResolutionError(Status error) {
      // Preserve addresses, because even old addresses may be used by the real policy
      this.error = error;
      initializeBalancingState();
    }

    private void initializeBalancingState() {
      if (updatedBalancingState) {
        return;
      }
      helper.updateBalancingState(ConnectivityState.IDLE, new LazyPicker());
      updatedBalancingState = true;
    }

    @Override
    public void requestConnection() {
      activate().requestConnection();
    }

    @Override
    public void shutdown() {
      delegate = new NoopLoadBalancer();
    }

    private final class LazyPicker extends SubchannelPicker {
      @Override
      public PickResult pickSubchannel(PickSubchannelArgs args) {
        // activate() is a no-op after shutdown()
        helper.getSynchronizationContext().execute(LazyDelegate.this::activate);
        return PickResult.withNoResult();
      }
    }
  }

  public static final class Factory extends LoadBalancer.Factory {
    private final LoadBalancer.Factory delegate;

    public Factory(LoadBalancer.Factory delegate) {
      this.delegate = Preconditions.checkNotNull(delegate, "delegate");
    }

    @Override public LoadBalancer newLoadBalancer(Helper helper) {
      return new LazyLoadBalancer(helper, delegate);
    }
  }

  private static final class NoopLoadBalancer extends LoadBalancer {
    @Override
    public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
      return Status.OK;
    }

    @Override
    public void handleNameResolutionError(Status error) {}

    @Override
    public void shutdown() {}
  }
}

1	/*
2	* Copyright 2024 The gRPC Authors
3	*
4	* Licensed under the Apache License, Version 2.0 (the "License");
5	* you may not use this file except in compliance with the License.
6	* You may obtain a copy of the License at
7	*
8	* http://www.apache.org/licenses/LICENSE-2.0
9	*
10	* Unless required by applicable law or agreed to in writing, software
11	* distributed under the License is distributed on an "AS IS" BASIS,
12	* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13	* See the License for the specific language governing permissions and
14	* limitations under the License.
15	*/
16
17	package io.grpc.xds;
18
19	import com.google.common.base.Preconditions;
20	import io.grpc.ConnectivityState;
21	import io.grpc.LoadBalancer;
22	import io.grpc.Status;
23	import io.grpc.util.ForwardingLoadBalancer;
24
25	/**
26	* A load balancer that starts in IDLE instead of CONNECTING. Once it starts connecting, it
27	* instantiates its delegate.
28	*/
29	final class LazyLoadBalancer extends ForwardingLoadBalancer {
30	private LoadBalancer delegate;
31
32	public LazyLoadBalancer(Helper helper, LoadBalancer.Factory delegateFactory) {	1✔
33	this.delegate = new LazyDelegate(helper, delegateFactory);	1✔
34	}	1✔
35
36	@Override
37	protected LoadBalancer delegate() {
38	return delegate;	1✔
39	}
40
41	@Override
42	public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
43	return delegate.acceptResolvedAddresses(resolvedAddresses);	1✔
44	}
45
46	private final class LazyDelegate extends LoadBalancer {
47	private final Helper helper;
48	private final LoadBalancer.Factory delegateFactory;
49	private ResolvedAddresses addresses;
50	private Status error;
51	private boolean updatedBalancingState;
52
53	public LazyDelegate(Helper helper, LoadBalancer.Factory delegateFactory) {	1✔
54	this.helper = Preconditions.checkNotNull(helper, "helper");	1✔
55	this.delegateFactory = Preconditions.checkNotNull(delegateFactory, "delegateFactory");	1✔
56	}	1✔
57
58	private LoadBalancer activate() {
59	if (delegate != this) {	1✔
60	return delegate;	1✔
61	}
62	delegate = delegateFactory.newLoadBalancer(helper);	1✔
63	if (addresses != null) {	1✔
64	delegate.acceptResolvedAddresses(addresses);	1✔
65	}
66	if (error != null) {	1✔
67	delegate.handleNameResolutionError(error);	×
68	}
69	return delegate;	1✔
70	}
71
72	@Override
73	public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
74	this.addresses = resolvedAddresses;	1✔
75	this.error = null;	1✔
76	initializeBalancingState();	1✔
77	return Status.OK;	1✔
78	}
79
80	@Override
81	public void handleNameResolutionError(Status error) {
82	// Preserve addresses, because even old addresses may be used by the real policy
83	this.error = error;	×
84	initializeBalancingState();	×
85	}	×
86
87	private void initializeBalancingState() {
88	if (updatedBalancingState) {	1✔
89	return;	×
90	}
91	helper.updateBalancingState(ConnectivityState.IDLE, new LazyPicker());	1✔
92	updatedBalancingState = true;	1✔
93	}	1✔
94
95	@Override
96	public void requestConnection() {
97	activate().requestConnection();	1✔
98	}	1✔
99
100	@Override
101	public void shutdown() {
102	delegate = new NoopLoadBalancer();	1✔
103	}	1✔
104
105	private final class LazyPicker extends SubchannelPicker {	1✔
106	@Override
107	public PickResult pickSubchannel(PickSubchannelArgs args) {
108	// activate() is a no-op after shutdown()
109	helper.getSynchronizationContext().execute(LazyDelegate.this::activate);	1✔
110	return PickResult.withNoResult();	1✔
111	}
112	}
113	}
114
115	public static final class Factory extends LoadBalancer.Factory {
116	private final LoadBalancer.Factory delegate;
117
118	public Factory(LoadBalancer.Factory delegate) {	1✔
119	this.delegate = Preconditions.checkNotNull(delegate, "delegate");	1✔
120	}	1✔
121
122	@Override public LoadBalancer newLoadBalancer(Helper helper) {
123	return new LazyLoadBalancer(helper, delegate);	1✔
124	}
125	}
126
127	private static final class NoopLoadBalancer extends LoadBalancer {
128	@Override
129	public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
130	return Status.OK;	×
131	}
132
133	@Override
134	public void handleNameResolutionError(Status error) {}	×
135
136	@Override
137	public void shutdown() {}	×
138	}
139	}

grpc / grpc-java / #19916

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous