From f21324c44135ab5657a9c51ee9855fac6d9e8ac0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 21 Oct 2025 18:47:49 +0200 Subject: [PATCH 1/3] Initial --- .../connectivity-apache-httpclient4/pom.xml | 4 ++ .../connectivity/AbstractHttpClientCache.java | 30 ++++++++++++++- .../AbstractHttpClientCacheTest.java | 38 +++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 cloudplatform/connectivity-apache-httpclient4/src/test/java/com/sap/cloud/sdk/cloudplatform/connectivity/AbstractHttpClientCacheTest.java 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..d061d35c6 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,22 @@ private Try tryGetOrCreateHttpClient( return Try.success(httpClient); } + private static boolean isHealthy( final HttpClient httpClient ) + throws IllegalArgumentException, + NullPointerException + { + 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()); + } +} From 23fafa582965142cc9d114eb2c4e0780cc088dcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 21 Oct 2025 18:56:23 +0200 Subject: [PATCH 2/3] Add sanity check --- .../cloudplatform/connectivity/AbstractHttpClientCache.java | 4 ++++ 1 file changed, 4 insertions(+) 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 d061d35c6..863924335 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 @@ -125,6 +125,10 @@ 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); From 4342aa16bfc3df90ce538eb036aec86eb7b2b653 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20D=C3=BCmont?= Date: Tue, 21 Oct 2025 18:59:25 +0200 Subject: [PATCH 3/3] Fix PMD --- .../sdk/cloudplatform/connectivity/AbstractHttpClientCache.java | 1 + 1 file changed, 1 insertion(+) 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 863924335..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 @@ -121,6 +121,7 @@ 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