diff --git a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java index 91007c392d1..f1981bcc223 100644 --- a/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java +++ b/rls/src/main/java/io/grpc/rls/CachingRlsLbClient.java @@ -111,6 +111,7 @@ final class CachingRlsLbClient { private final RouteLookupServiceStub rlsStub; private final RlsPicker rlsPicker; private final ResolvedAddressFactory childLbResolvedAddressFactory; + @GuardedBy("lock") private final RefCountedChildPolicyWrapperFactory refCountedChildPolicyWrapperFactory; private final ChannelLogger logger; diff --git a/rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java b/rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java index 37b7c2eb0be..be067732bac 100644 --- a/rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java +++ b/rls/src/main/java/io/grpc/rls/LbPolicyConfiguration.java @@ -41,7 +41,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.concurrent.atomic.AtomicLong; import javax.annotation.Nullable; /** Configuration for RLS load balancing policy. */ @@ -203,9 +202,8 @@ public String toString() { } } - /** Factory for {@link ChildPolicyWrapper}. */ + /** Factory for {@link ChildPolicyWrapper}. Not thread-safe. */ static final class RefCountedChildPolicyWrapperFactory { - // GuardedBy CachingRlsLbClient.lock @VisibleForTesting final Map childPolicyMap = new HashMap<>(); @@ -227,7 +225,6 @@ public RefCountedChildPolicyWrapperFactory( this.childLbStatusListener = checkNotNull(childLbStatusListener, "childLbStatusListener"); } - // GuardedBy CachingRlsLbClient.lock ChildPolicyWrapper createOrGet(String target) { // TODO(creamsoup) check if the target is valid or not RefCountedChildPolicyWrapper pooledChildPolicyWrapper = childPolicyMap.get(target); @@ -247,7 +244,6 @@ ChildPolicyWrapper createOrGet(String target) { } } - // GuardedBy CachingRlsLbClient.lock List createOrGet(List targets) { List retVal = new ArrayList<>(); for (String target : targets) { @@ -256,7 +252,6 @@ List createOrGet(List targets) { return retVal; } - // GuardedBy CachingRlsLbClient.lock void release(ChildPolicyWrapper childPolicyWrapper) { checkNotNull(childPolicyWrapper, "childPolicyWrapper"); String target = childPolicyWrapper.getTarget(); @@ -402,7 +397,7 @@ interface ChildLbStatusListener { private static final class RefCountedChildPolicyWrapper implements ObjectPool { - private final AtomicLong refCnt = new AtomicLong(); + private long refCnt; @Nullable private ChildPolicyWrapper childPolicyWrapper; @@ -413,7 +408,7 @@ private RefCountedChildPolicyWrapper(ChildPolicyWrapper childPolicyWrapper) { @Override public ChildPolicyWrapper getObject() { checkState(!isReleased(), "ChildPolicyWrapper is already released"); - refCnt.getAndIncrement(); + refCnt++; return childPolicyWrapper; } @@ -426,7 +421,7 @@ public ChildPolicyWrapper returnObject(Object object) { checkState( childPolicyWrapper == object, "returned object doesn't match the pooled childPolicyWrapper"); - long newCnt = refCnt.decrementAndGet(); + long newCnt = --refCnt; checkState(newCnt != -1, "Cannot return never pooled childPolicyWrapper"); if (newCnt == 0) { childPolicyWrapper.shutdown(); @@ -447,7 +442,7 @@ static RefCountedChildPolicyWrapper of(ChildPolicyWrapper childPolicyWrapper) { public String toString() { return MoreObjects.toStringHelper(this) .add("object", childPolicyWrapper) - .add("refCnt", refCnt.get()) + .add("refCnt", refCnt) .toString(); } }