diff --git a/cloudplatform/connectivity-apache-httpclient4/pom.xml b/cloudplatform/connectivity-apache-httpclient4/pom.xml index 31041e446..ec1d84bdf 100644 --- a/cloudplatform/connectivity-apache-httpclient4/pom.xml +++ b/cloudplatform/connectivity-apache-httpclient4/pom.xml @@ -96,6 +96,10 @@ + + org.apache.commons + commons-lang3 + org.projectlombok diff --git a/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCache.java b/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCache.java index 29c336cf0..5be165967 100644 --- a/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCache.java +++ b/cloudplatform/connectivity-apache-httpclient4/src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCache.java @@ -6,7 +6,11 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.http.client.HttpClient; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.apache.http.pool.AbstractConnPool; import com.github.benmanes.caffeine.cache.Cache; import com.sap.cloud.sdk.cloudplatform.cache.CacheKey; @@ -14,6 +18,7 @@ import io.vavr.control.Try; import lombok.extern.slf4j.Slf4j; +import lombok.val; /** * Provides caching functionality to the {@code HttpClientAccessor}. @@ -89,9 +94,16 @@ private Try tryGetOrCreateHttpClient( final Cache cache = maybeCache.get(); final CacheKey cacheKey = maybeKey.get(); - final HttpClient httpClient; + HttpClient httpClient; try { httpClient = cache.get(cacheKey, anyKey -> createHttpClient.get()); + + if( !isHealthy(httpClient) ) { + log.warn("The HttpClient retrieved from the cache has a shutdown connection pool."); + cache.invalidate(cacheKey); + httpClient = cache.get(cacheKey, anyKey -> createHttpClient.get()); + } + Objects .requireNonNull( httpClient, @@ -109,6 +121,27 @@ private Try tryGetOrCreateHttpClient( return Try.success(httpClient); } + @SuppressWarnings( "PMD.CloseResource" ) // no new resource is opened + private static boolean isHealthy( final HttpClient httpClient ) + throws IllegalArgumentException, + NullPointerException + { + if( !(httpClient instanceof HttpClientWrapper) ) { + log.trace("Cannot verify health of HttpClient: {}", httpClient); + return true; + } + try { + val hc = (CloseableHttpClient) FieldUtils.readField(httpClient, "httpClient", true); + val cm = (PoolingHttpClientConnectionManager) FieldUtils.readField(hc, "connManager", true); + val cp = (AbstractConnPool) FieldUtils.readField(cm, "pool", true); + return !cp.isShutdown(); + } + catch( final Exception e ) { + log.warn("Failed to access connection manager.", e); + return true; + } + } + /** * Getter for the cache to be used. *

diff --git a/cloudplatform/connectivity-apache-httpclient4/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCacheTest.java b/cloudplatform/connectivity-apache-httpclient4/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCacheTest.java new file mode 100644 index 000000000..dc290ff32 --- /dev/null +++ b/cloudplatform/connectivity-apache-httpclient4/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCacheTest.java @@ -0,0 +1,38 @@ +package com.sap.cloud.sdk.cloudplatform.connectivity; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.apache.http.client.methods.HttpHead; +import org.apache.http.impl.client.CloseableHttpClient; +import org.junit.jupiter.api.Test; + +import lombok.SneakyThrows; +import lombok.val; + +public class AbstractHttpClientCacheTest +{ + @SneakyThrows + @Test + void testClosedPool() + { + val d = DefaultHttpDestination.builder("https://sap.com").build(); + + val httpClient1 = HttpClientAccessor.getHttpClient(d); + httpClient1.execute(new HttpHead()); + httpClient1.execute(new HttpHead()); + + ((CloseableHttpClient) httpClient1).close(); + assertThatThrownBy(() -> httpClient1.execute(new HttpHead())) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Connection pool shut down"); + assertThatThrownBy(() -> httpClient1.execute(new HttpHead())) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Connection pool shut down"); + + val httpClient2 = HttpClientAccessor.getHttpClient(d); + assertThat(httpClient1).isNotSameAs(httpClient2); + httpClient2.execute(new HttpHead()); + httpClient2.execute(new HttpHead()); + } +}