Describe the Bug
The current implementation of caching and evicting an HTTP Client lead to not closed HttpClients.
|
DefaultApacheHttpClient5Cache( @Nonnull final Duration cacheDuration, @Nonnull final Ticker ticker ) |
|
{ |
|
cache = Caffeine.newBuilder().expireAfterAccess(cacheDuration).ticker(ticker).build(); |
|
CacheManager.register(cache); |
|
} |
|
|
Technically the HttpClients are CloseableHttpClient and should / need(specifically for pooled connections) to be closed after usage.
(See also this discussion)
In the current code-base, when a HttpClient with a PoolingHttpClientConnectionManager is created, it's not cleaned up properly on eviction.
The following workaround does not work in all cases:
cache = Caffeine.newBuilder().expireAfterAccess(duration, unit).ticker(ticker).evictionListener((key, value, cause) -> {
if (value instanceof CloseableHttpClient closeableHttpClient) {
try {
closeableHttpClient.close();
} catch (final Exception e) {
log.warn("Failed to close HttpClient. Ignoring the exception and continue.", e);
}
}
}).build();
There are two basic scenarios:
- The client was created and is no longer in use and the eviction time triggers the cleanup. ✅
- The client was created and in still in use (long running operation, async operation, ...) and the eviction time triggers the cleanup. Then the
evictionListener would kill the connection underneath. ❌
Steps to Reproduce
Code review.
Expected Behavior
Proper closing of HttpClients.
Screenshots
No response
Used Versions
Current state in main.
Code Examples
Stack Trace
No response
Log File
Log file
...
Affected Development Phase
Development
Impact
No Impact
Timeline
No response
Describe the Bug
The current implementation of caching and evicting an HTTP Client lead to not closed
HttpClients.cloud-sdk-java/cloudplatform/connectivity-apache-httpclient5/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/DefaultApacheHttpClient5Cache.java
Lines 42 to 47 in 4fb1f6a
Technically the
HttpClientsareCloseableHttpClientand should / need(specifically for pooled connections) to be closed after usage.(See also this discussion)
In the current code-base, when a
HttpClientwith aPoolingHttpClientConnectionManageris created, it's not cleaned up properly on eviction.The following workaround does not work in all cases:
There are two basic scenarios:
evictionListenerwould kill the connection underneath. ❌Steps to Reproduce
Code review.
Expected Behavior
Proper closing of HttpClients.
Screenshots
No response
Used Versions
Current state in main.
Code Examples
// Your code hereStack Trace
No response
Log File
Log file
...Affected Development Phase
Development
Impact
No Impact
Timeline
No response