Skip to content

Commit

Permalink
xds:Fix ConcurrentModificationException in PriorityLoadBalancer (#9728)
Browse files Browse the repository at this point in the history
Fix ConcurrentModificationException in PriorityLoadBalancer by making copy of children values to iterate rather than directly using children in for loop.
  • Loading branch information
larry-safran authored Dec 2, 2022
1 parent 79f4411 commit 3e5fa7c
Showing 1 changed file with 17 additions and 6 deletions.
23 changes: 17 additions & 6 deletions xds/src/main/java/io/grpc/xds/PriorityLoadBalancer.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import io.grpc.xds.PriorityLoadBalancerProvider.PriorityLbConfig.PriorityChildConfig;
import io.grpc.xds.XdsLogger.XdsLogLevel;
import io.grpc.xds.XdsSubchannelPickers.ErrorPicker;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand All @@ -59,6 +61,8 @@ final class PriorityLoadBalancer extends LoadBalancer {

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

// Following fields are only null initially.
Expand Down Expand Up @@ -91,15 +95,20 @@ public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
priorityNames = config.priorities;
priorityConfigs = config.childConfigs;
Set<String> prioritySet = new HashSet<>(config.priorities);
for (String priority : children.keySet()) {
ArrayList<String> childKeys = new ArrayList<>(children.keySet());
for (String priority : childKeys) {
if (!prioritySet.contains(priority)) {
children.get(priority).deactivate();
ChildLbState childLbState = children.get(priority);
if (childLbState != null) {
childLbState.deactivate();
}
}
}
handlingResolvedAddresses = true;
for (String priority : priorityNames) {
if (children.containsKey(priority)) {
children.get(priority).updateResolvedAddresses();
ChildLbState childLbState = children.get(priority);
if (childLbState != null) {
childLbState.updateResolvedAddresses();
}
}
handlingResolvedAddresses = false;
Expand All @@ -111,7 +120,8 @@ public boolean acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
public void handleNameResolutionError(Status error) {
logger.log(XdsLogLevel.WARNING, "Received name resolution error: {0}", error);
boolean gotoTransientFailure = true;
for (ChildLbState child : children.values()) {
Collection<ChildLbState> childValues = new ArrayList<>(children.values());
for (ChildLbState child : childValues) {
if (priorityNames.contains(child.priority)) {
child.lb.handleNameResolutionError(error);
gotoTransientFailure = false;
Expand All @@ -125,7 +135,8 @@ public void handleNameResolutionError(Status error) {
@Override
public void shutdown() {
logger.log(XdsLogLevel.INFO, "Shutdown");
for (ChildLbState child : children.values()) {
Collection<ChildLbState> childValues = new ArrayList<>(children.values());
for (ChildLbState child : childValues) {
child.tearDown();
}
children.clear();
Expand Down

0 comments on commit 3e5fa7c

Please sign in to comment.