Description
Hello,
Recently we discovered a thread leak in the HttpProjectConfigManager.java which forgets to override the "close()" method and therefore to close the ClosableHttpClient (https://github.com/optimizely/java-sdk/blob/master/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java#L64).
private final OptimizelyHttpClient httpClient;
private final URI uri;
private final String datafileAccessToken;
private String datafileLastModified;
private HttpProjectConfigManager(long period,
TimeUnit timeUnit,
OptimizelyHttpClient httpClient,
String url,
String datafileAccessToken,
long blockingTimeoutPeriod,
TimeUnit blockingTimeoutUnit,
NotificationCenter notificationCenter) {
super(period, timeUnit, blockingTimeoutPeriod, blockingTimeoutUnit, notificationCenter);
this.httpClient = httpClient;
this.uri = URI.create(url);
this.datafileAccessToken = datafileAccessToken;
}
If the httpClient is built by the Optimizely code (https://github.com/optimizely/java-sdk/blob/master/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java#L331) then the eviction configuration instructs the Apache httpClient to start a Connection evictor thread which won't be interrupted on shutdown.
if (httpClient == null) {
httpClient = OptimizelyHttpClient.builder()
.withEvictIdleConnections(evictConnectionIdleTimePeriod, evictConnectionIdleTimeUnit)
.build();
}
Currently only the parent PollingProjectConfigManager.java calls "close()" to free up resources (https://github.com/optimizely/java-sdk/blob/master/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java#L210).
public synchronized void close() {
stop();
scheduledExecutorService.shutdownNow();
started = false;
}
Expected behaviour would be to override the "close()" method in HttpProjectConfigManager.java, call "httpClient.close()" and continue the flow to the parent class (PollingProjectConfigManager.java#L210) to close its resources.