Skip to content

Commit 864b1b0

Browse files
committed
Merge branch 'sauravzg-feat-cached-channel' into ext-proc
# Conflicts: # xds/src/main/java/io/grpc/xds/internal/grpcservice/CachedChannelManager.java # xds/src/test/java/io/grpc/xds/internal/grpcservice/CachedChannelManagerTest.java
2 parents 7d292ee + 997c65d commit 864b1b0

2 files changed

Lines changed: 55 additions & 6 deletions

File tree

xds/src/main/java/io/grpc/xds/internal/grpcservice/CachedChannelManager.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.base.Preconditions.checkNotNull;
2020

2121
import com.google.auto.value.AutoValue;
22+
import com.google.common.annotations.VisibleForTesting;
2223
import io.grpc.ManagedChannel;
2324
import io.grpc.xds.internal.grpcservice.GrpcServiceConfig.GoogleGrpcConfig;
2425
import java.util.concurrent.atomic.AtomicReference;
@@ -32,6 +33,7 @@ public class CachedChannelManager {
3233
private final Object lock = new Object();
3334

3435
private final AtomicReference<ChannelHolder> channelHolder = new AtomicReference<>();
36+
private boolean closed;
3537

3638
/**
3739
* Default constructor for production that creates a channel using the config's target and
@@ -48,6 +50,7 @@ public CachedChannelManager() {
4850
/**
4951
* Constructor for testing to inject a channel creator.
5052
*/
53+
@VisibleForTesting
5154
public CachedChannelManager(Function<GrpcServiceConfig, ManagedChannel> channelCreator) {
5255
this.channelCreator = checkNotNull(channelCreator, "channelCreator");
5356
}
@@ -73,6 +76,9 @@ public ManagedChannel getChannel(GrpcServiceConfig config) {
7376

7477
// 2. Slow path: Update with locking
7578
synchronized (lock) {
79+
if (closed) {
80+
throw new IllegalStateException("CachedChannelManager is closed");
81+
}
7682
holder = channelHolder.get(); // Double check
7783
if (holder != null && holder.channelKey().equals(newChannelKey)) {
7884
return holder.channel();
@@ -98,9 +104,12 @@ public ManagedChannel getChannel(GrpcServiceConfig config) {
98104

99105
/** Removes underlying resources on shutdown. */
100106
public void close() {
101-
ChannelHolder holder = channelHolder.get();
102-
if (holder != null) {
103-
holder.channel().shutdown();
107+
synchronized (lock) {
108+
closed = true;
109+
ChannelHolder holder = channelHolder.getAndSet(null);
110+
if (holder != null) {
111+
holder.channel().shutdown();
112+
}
104113
}
105114
}
106115

xds/src/test/java/io/grpc/xds/internal/grpcservice/CachedChannelManagerTest.java

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,15 +68,15 @@ public void setUp() {
6868
private GrpcServiceConfig buildConfig(String target, String credsType) {
6969
ChannelCredsConfig credsConfig = mock(ChannelCredsConfig.class);
7070
when(credsConfig.type()).thenReturn(credsType);
71-
71+
7272
ConfiguredChannelCredentials creds = ConfiguredChannelCredentials.create(
7373
mock(io.grpc.ChannelCredentials.class), credsConfig);
74-
74+
7575
GoogleGrpcConfig googleGrpc = GoogleGrpcConfig.builder()
7676
.target(target)
7777
.configuredChannelCredentials(creds)
7878
.build();
79-
79+
8080
return GrpcServiceConfig.builder()
8181
.googleGrpc(googleGrpc)
8282
.initialMetadata(ImmutableList.of())
@@ -120,4 +120,44 @@ public void close_shutsDownChannel() {
120120

121121
verify(mockChannel1).shutdown();
122122
}
123+
124+
@Test
125+
public void getChannel_afterClose_throwsException() {
126+
when(mockCreator.apply(config1)).thenReturn(mockChannel1);
127+
128+
manager.getChannel(config1);
129+
manager.close();
130+
131+
try {
132+
manager.getChannel(config1);
133+
org.junit.Assert.fail("Expected IllegalStateException");
134+
} catch (IllegalStateException e) {
135+
assertThat(e).hasMessageThat().contains("CachedChannelManager is closed");
136+
}
137+
}
138+
139+
@Test
140+
public void constructor_defaultCreatesChannel() {
141+
CachedChannelManager defaultManager = new CachedChannelManager();
142+
io.grpc.ChannelCredentials creds = io.grpc.InsecureChannelCredentials.create();
143+
ChannelCredsConfig credsConfig = mock(ChannelCredsConfig.class);
144+
when(credsConfig.type()).thenReturn("insecure");
145+
ConfiguredChannelCredentials configuredCreds =
146+
ConfiguredChannelCredentials.create(creds, credsConfig);
147+
GoogleGrpcConfig googleGrpc = GoogleGrpcConfig.builder()
148+
.target("localhost:8080")
149+
.configuredChannelCredentials(configuredCreds)
150+
.build();
151+
GrpcServiceConfig config = GrpcServiceConfig.builder()
152+
.googleGrpc(googleGrpc)
153+
.initialMetadata(ImmutableList.of())
154+
.build();
155+
156+
ManagedChannel channel = defaultManager.getChannel(config);
157+
assertThat(channel).isNotNull();
158+
159+
channel.shutdownNow();
160+
defaultManager.close();
161+
}
162+
123163
}

0 commit comments

Comments
 (0)