Skip to content

Commit eec9f67

Browse files
authored
xds:Fix ConcurrentModificationException in PriorityLoadBalancer (#9728) (#9744)
Fix ConcurrentModificationException in PriorityLoadBalancer by making copy of children values to iterate rather than directly using children in for loop.
1 parent 73c4194 commit eec9f67

File tree

1 file changed

+17
-6
lines changed

1 file changed

+17
-6
lines changed

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

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig.PriorityChildConfig;
3838
import io.grpc.xds.XdsLogger.XdsLogLevel;
3939
import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
40+
import java.util.ArrayList;
41+
import java.util.Collection;
4042
import java.util.HashMap;
4143
import java.util.HashSet;
4244
import java.util.List;
@@ -59,6 +61,8 @@ final class PriorityLoadBalancer extends LoadBalancer {
5961

6062
// Includes all active and deactivated children. Mutable. New entries are only added from priority
6163
// 0 up to the selected priority. An entry is only deleted 15 minutes after its deactivation.
64+
// Note that because all configuration updates should be atomic, updates to children can happen
65+
// outside of the synchronization context. Therefore copy values before looping over them.
6266
private final Map<String, ChildLbState> children = new HashMap<>();
6367

6468
// Following fields are only null initially.
@@ -91,15 +95,20 @@ public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
9195
priorityNames = config.priorities;
9296
priorityConfigs = config.childConfigs;
9397
Set<String> prioritySet = new HashSet<>(config.priorities);
94-
for (String priority : children.keySet()) {
98+
ArrayList<String> childKeys = new ArrayList<>(children.keySet());
99+
for (String priority : childKeys) {
95100
if (!prioritySet.contains(priority)) {
96-
children.get(priority).deactivate();
101+
ChildLbState childLbState = children.get(priority);
102+
if (childLbState != null) {
103+
childLbState.deactivate();
104+
}
97105
}
98106
}
99107
handlingResolvedAddresses = true;
100108
for (String priority : priorityNames) {
101-
if (children.containsKey(priority)) {
102-
children.get(priority).updateResolvedAddresses();
109+
ChildLbState childLbState = children.get(priority);
110+
if (childLbState != null) {
111+
childLbState.updateResolvedAddresses();
103112
}
104113
}
105114
handlingResolvedAddresses = false;
@@ -111,7 +120,8 @@ public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
111120
public void handleNameResolutionError(Status error) {
112121
logger.log(XdsLogLevel.WARNING, "Received name resolution error: {0}", error);
113122
boolean gotoTransientFailure = true;
114-
for (ChildLbState child : children.values()) {
123+
Collection<ChildLbState> childValues = new ArrayList<>(children.values());
124+
for (ChildLbState child : childValues) {
115125
if (priorityNames.contains(child.priority)) {
116126
child.lb.handleNameResolutionError(error);
117127
gotoTransientFailure = false;
@@ -125,7 +135,8 @@ public void handleNameResolutionError(Status error) {
125135
@Override
126136
public void shutdown() {
127137
logger.log(XdsLogLevel.INFO, "Shutdown");
128-
for (ChildLbState child : children.values()) {
138+
Collection<ChildLbState> childValues = new ArrayList<>(children.values());
139+
for (ChildLbState child : childValues) {
129140
child.tearDown();
130141
}
131142
children.clear();

0 commit comments

Comments
 (0)