Skip to content

Commit de1daa0

Browse files
committed
channelfactory does not shutdown
1 parent ef01f2a commit de1daa0

File tree

6 files changed

+13
-71
lines changed

6 files changed

+13
-71
lines changed

openfeature-provider/java/src/main/java/com/spotify/confidence/ChannelFactory.java

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,8 @@
2222
* <li>Debugging: add custom logging or tracing interceptors
2323
* </ul>
2424
*
25-
* <p><strong>Lifecycle:</strong> The factory is responsible for managing the lifecycle of all
26-
* channels it creates. When {@link #shutdown()} is called, it should shut down all channels that
27-
* were created via {@link #create(String, List)}.
25+
* <p><strong>Channel Lifecycle:</strong> Channels created by this factory must be shut down by the
26+
* caller when they are no longer needed. The factory does not manage channel lifecycles.
2827
*/
2928
public interface ChannelFactory {
3029
/**
@@ -35,13 +34,4 @@ public interface ChannelFactory {
3534
* @return a configured ManagedChannel
3635
*/
3736
ManagedChannel create(String target, List<ClientInterceptor> defaultInterceptors);
38-
39-
/**
40-
* Shuts down all channels created by this factory. This method should be called when the provider
41-
* is shutting down to ensure proper resource cleanup.
42-
*
43-
* <p>Implementations should shut down all channels that were created via {@link #create(String,
44-
* List)} and wait for them to terminate gracefully.
45-
*/
46-
void shutdown();
4737
}

openfeature-provider/java/src/main/java/com/spotify/confidence/DefaultChannelFactory.java

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,6 @@
77
import java.util.ArrayList;
88
import java.util.List;
99
import java.util.Optional;
10-
import java.util.concurrent.CopyOnWriteArrayList;
11-
import java.util.concurrent.TimeUnit;
12-
import org.slf4j.Logger;
13-
import org.slf4j.LoggerFactory;
1410

1511
/**
1612
* Default implementation of ChannelFactory that creates standard gRPC channels with security
@@ -22,13 +18,9 @@
2218
* <li>Uses TLS by default, unless CONFIDENCE_GRPC_PLAINTEXT=true
2319
* <li>Adds a default deadline interceptor (1 minute timeout)
2420
* <li>Applies any additional interceptors passed via defaultInterceptors
25-
* <li>Tracks all created channels and shuts them down when {@link #shutdown()} is called
2621
* </ul>
2722
*/
2823
public class DefaultChannelFactory implements ChannelFactory {
29-
private static final Logger logger = LoggerFactory.getLogger(DefaultChannelFactory.class);
30-
private final List<ManagedChannel> channels = new CopyOnWriteArrayList<>();
31-
3224
@Override
3325
public ManagedChannel create(String target, List<ClientInterceptor> defaultInterceptors) {
3426
final boolean useGrpcPlaintext =
@@ -45,28 +37,6 @@ public ManagedChannel create(String target, List<ClientInterceptor> defaultInter
4537
List<ClientInterceptor> allInterceptors = new ArrayList<>(defaultInterceptors);
4638
allInterceptors.add(new DefaultDeadlineClientInterceptor(Duration.ofMinutes(1)));
4739

48-
ManagedChannel channel =
49-
builder.intercept(allInterceptors.toArray(new ClientInterceptor[0])).build();
50-
channels.add(channel);
51-
return channel;
52-
}
53-
54-
@Override
55-
public void shutdown() {
56-
logger.debug("Shutting down {} channels created by DefaultChannelFactory", channels.size());
57-
for (ManagedChannel channel : channels) {
58-
try {
59-
channel.shutdown();
60-
if (!channel.awaitTermination(5, TimeUnit.SECONDS)) {
61-
logger.warn("Channel did not terminate within 5 seconds, forcing shutdown");
62-
channel.shutdownNow();
63-
}
64-
} catch (InterruptedException e) {
65-
logger.warn("Interrupted while shutting down channel", e);
66-
channel.shutdownNow();
67-
Thread.currentThread().interrupt();
68-
}
69-
}
70-
channels.clear();
40+
return builder.intercept(allInterceptors.toArray(new ClientInterceptor[0])).build();
7141
}
7242
}

openfeature-provider/java/src/main/java/com/spotify/confidence/GrpcWasmFlagLogger.java

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,8 @@
55
import com.google.common.annotations.VisibleForTesting;
66
import com.spotify.confidence.flags.resolver.v1.InternalFlagLoggerServiceGrpc;
77
import com.spotify.confidence.flags.resolver.v1.WriteFlagLogsRequest;
8-
import io.grpc.CallOptions;
9-
import io.grpc.Channel;
10-
import io.grpc.ClientCall;
11-
import io.grpc.ClientInterceptor;
12-
import io.grpc.ForwardingClientCall;
13-
import io.grpc.Metadata;
14-
import io.grpc.MethodDescriptor;
8+
import io.grpc.*;
9+
1510
import java.time.Duration;
1611
import java.util.ArrayList;
1712
import java.util.List;
@@ -35,6 +30,7 @@ public class GrpcWasmFlagLogger implements WasmFlagLogger {
3530
private final ExecutorService executorService;
3631
private final FlagLogWriter writer;
3732
private final Duration shutdownTimeout;
33+
private ManagedChannel channel;
3834

3935
@VisibleForTesting
4036
public GrpcWasmFlagLogger(String clientSecret, FlagLogWriter writer) {
@@ -71,9 +67,9 @@ public GrpcWasmFlagLogger(String clientSecret, ChannelFactory channelFactory) {
7167
});
7268
}
7369

74-
private static InternalFlagLoggerServiceGrpc.InternalFlagLoggerServiceBlockingStub createAuthStub(
75-
ChannelFactory channelFactory, String clientSecret) {
76-
final var channel = createConfidenceChannel(channelFactory);
70+
private InternalFlagLoggerServiceGrpc.InternalFlagLoggerServiceBlockingStub createAuthStub(
71+
ChannelFactory channelFactory, String clientSecret) {
72+
this.channel = createConfidenceChannel(channelFactory);
7773
return addAuthInterceptor(InternalFlagLoggerServiceGrpc.newBlockingStub(channel), clientSecret);
7874
}
7975

@@ -181,6 +177,9 @@ private void sendSync(WriteFlagLogsRequest request) {
181177
*/
182178
@Override
183179
public void shutdown() {
180+
if (channel != null) {
181+
channel.shutdown();
182+
}
184183
executorService.shutdown();
185184
try {
186185
if (!executorService.awaitTermination(shutdownTimeout.toMillis(), TimeUnit.MILLISECONDS)) {

openfeature-provider/java/src/main/java/com/spotify/confidence/OpenFeatureLocalResolveProvider.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,9 +315,6 @@ public void shutdown() {
315315
// writes
316316
this.wasmResolveApi.close();
317317

318-
if (this.channelFactory != null) {
319-
this.channelFactory.shutdown();
320-
}
321318
FeatureProvider.super.shutdown();
322319
}
323320

openfeature-provider/java/src/test/java/com/spotify/confidence/ChannelFactoryTest.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,6 @@ public ManagedChannel create(String target, List<ClientInterceptor> interceptors
3434
}
3535
return builder.build();
3636
}
37-
38-
@Override
39-
public void shutdown() {
40-
// Test implementation - no-op
41-
}
4237
};
4338

4439
new OpenFeatureLocalResolveProvider(new LocalProviderConfig(customFactory), "clientsecret");

openfeature-provider/java/src/test/java/com/spotify/confidence/OpenFeatureLocalResolveProviderIntegrationTest.java

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -112,22 +112,13 @@ void setUp() throws Exception {
112112
// Create custom channel factory that connects to in-process server
113113
final ChannelFactory testChannelFactory =
114114
new ChannelFactory() {
115-
private final List<ManagedChannel> channels = new ArrayList<>();
116-
117115
@Override
118116
public ManagedChannel create(String target, List<ClientInterceptor> interceptors) {
119117
InProcessChannelBuilder builder = InProcessChannelBuilder.forName(serverName);
120118
if (!interceptors.isEmpty()) {
121119
builder.intercept(interceptors.toArray(new ClientInterceptor[0]));
122120
}
123-
ManagedChannel channel = builder.build();
124-
this.channels.add(channel);
125-
return channel;
126-
}
127-
128-
@Override
129-
public void shutdown() {
130-
channels.stream().forEach(ManagedChannel::shutdown);
121+
return builder.build();
131122
}
132123
};
133124

0 commit comments

Comments
 (0)