Skip to content

Commit 4530bdd

Browse files
committed
Fixup 12690: Addres copilot comments
1 parent 0f53469 commit 4530bdd

File tree

2 files changed

+25
-3
lines changed

2 files changed

+25
-3
lines changed

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public class CachedChannelManager {
3333
private final Object lock = new Object();
3434

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

3738
/**
3839
* Default constructor for production that creates a channel using the config's target and
@@ -75,6 +76,9 @@ public ManagedChannel getChannel(GrpcServiceConfig config) {
7576

7677
// 2. Slow path: Update with locking
7778
synchronized (lock) {
79+
if (closed) {
80+
throw new IllegalStateException("CachedChannelManager is closed");
81+
}
7882
holder = channelHolder.get(); // Double check
7983
if (holder != null && holder.channelKey().equals(newChannelKey)) {
8084
return holder.channel();
@@ -100,9 +104,12 @@ public ManagedChannel getChannel(GrpcServiceConfig config) {
100104

101105
/** Removes underlying resources on shutdown. */
102106
public void close() {
103-
ChannelHolder holder = channelHolder.get();
104-
if (holder != null) {
105-
holder.channel().shutdown();
107+
synchronized (lock) {
108+
closed = true;
109+
ChannelHolder holder = channelHolder.getAndSet(null);
110+
if (holder != null) {
111+
holder.channel().shutdown();
112+
}
106113
}
107114
}
108115

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,19 @@ 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+
}
123138
}

0 commit comments

Comments
 (0)