generated from SAP/repository-template
-
Notifications
You must be signed in to change notification settings - Fork 20
Open
Labels
Description
Describe the Bug
The current implementation of caching and evicting an HTTP Client lead to not closed HttpClients
.
Lines 42 to 47 in 4fb1f6a
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
// Your code here
Stack Trace
No response
Log File
Log file
...Affected Development Phase
Development
Impact
No Impact
Timeline
No response