Skip to content

Commit cd21135

Browse files
authored
Merge pull request #3727 from JoeCqupt/fix-connection-leak
fix httpclient5.x connection leak
2 parents d1af099 + 4ff93c5 commit cd21135

File tree

9 files changed

+272
-0
lines changed

9 files changed

+272
-0
lines changed
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
3+
xmlns="http://maven.apache.org/POM/4.0.0"
4+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
5+
<modelVersion>4.0.0</modelVersion>
6+
7+
<artifactId>httpclient</artifactId>
8+
<packaging>jar</packaging>
9+
10+
<name>Spring Cloud Gateway HttpClient Integration Test</name>
11+
<description>Spring Cloud Gateway HttpClient Integration Test</description>
12+
13+
<properties>
14+
</properties>
15+
16+
<parent>
17+
<groupId>org.springframework.cloud</groupId>
18+
<artifactId>spring-cloud-gateway-integration-tests</artifactId>
19+
<version>4.1.8-SNAPSHOT</version>
20+
<relativePath>..</relativePath> <!-- lookup parent from repository -->
21+
</parent>
22+
23+
24+
<dependencies>
25+
<dependency>
26+
<groupId>org.springframework.boot</groupId>
27+
<artifactId>spring-boot-starter-web</artifactId>
28+
</dependency>
29+
30+
<dependency>
31+
<groupId>org.springframework.boot</groupId>
32+
<artifactId>spring-boot-starter-webflux</artifactId>
33+
</dependency>
34+
35+
<dependency>
36+
<groupId>org.springframework.cloud</groupId>
37+
<artifactId>spring-cloud-starter-gateway-mvc</artifactId>
38+
</dependency>
39+
40+
<dependency>
41+
<groupId>org.springframework.retry</groupId>
42+
<artifactId>spring-retry</artifactId>
43+
</dependency>
44+
45+
<dependency>
46+
<groupId>org.springframework.cloud</groupId>
47+
<artifactId>spring-cloud-starter-loadbalancer</artifactId>
48+
</dependency>
49+
50+
<dependency>
51+
<groupId>org.apache.httpcomponents.client5</groupId>
52+
<artifactId>httpclient5</artifactId>
53+
</dependency>
54+
55+
<dependency>
56+
<groupId>org.springframework.boot</groupId>
57+
<artifactId>spring-boot-starter-test</artifactId>
58+
<scope>test</scope>
59+
</dependency>
60+
<dependency>
61+
<groupId>org.assertj</groupId>
62+
<artifactId>assertj-core</artifactId>
63+
<scope>test</scope>
64+
</dependency>
65+
</dependencies>
66+
67+
<build>
68+
<plugins>
69+
<plugin>
70+
<artifactId>maven-deploy-plugin</artifactId>
71+
<configuration>
72+
<skip>true</skip>
73+
</configuration>
74+
</plugin>
75+
</plugins>
76+
</build>
77+
</project>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*
2+
* Copyright 2013-2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cloud.gateway.tests.httpclient;
18+
19+
import java.time.Duration;
20+
import java.util.concurrent.ConcurrentHashMap;
21+
import java.util.concurrent.atomic.AtomicInteger;
22+
23+
import org.apache.commons.logging.Log;
24+
import org.apache.commons.logging.LogFactory;
25+
import org.apache.hc.client5.http.config.RequestConfig;
26+
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
27+
import org.apache.hc.client5.http.impl.classic.HttpClients;
28+
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
29+
import org.apache.hc.core5.util.Timeout;
30+
31+
import org.springframework.beans.factory.annotation.Value;
32+
import org.springframework.boot.SpringApplication;
33+
import org.springframework.boot.SpringBootConfiguration;
34+
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
35+
import org.springframework.cloud.client.DefaultServiceInstance;
36+
import org.springframework.cloud.loadbalancer.annotation.LoadBalancerClient;
37+
import org.springframework.cloud.loadbalancer.core.ServiceInstanceListSupplier;
38+
import org.springframework.cloud.loadbalancer.support.ServiceInstanceListSuppliers;
39+
import org.springframework.context.annotation.Bean;
40+
import org.springframework.http.HttpStatus;
41+
import org.springframework.http.ResponseEntity;
42+
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
43+
import org.springframework.web.bind.annotation.GetMapping;
44+
import org.springframework.web.bind.annotation.RequestParam;
45+
import org.springframework.web.bind.annotation.RestController;
46+
import org.springframework.web.servlet.function.RouterFunction;
47+
import org.springframework.web.servlet.function.ServerResponse;
48+
49+
import static org.springframework.cloud.gateway.server.mvc.filter.FilterFunctions.prefixPath;
50+
import static org.springframework.cloud.gateway.server.mvc.filter.LoadBalancerFilterFunctions.lb;
51+
import static org.springframework.cloud.gateway.server.mvc.filter.RetryFilterFunctions.retry;
52+
import static org.springframework.cloud.gateway.server.mvc.handler.GatewayRouterFunctions.route;
53+
import static org.springframework.cloud.gateway.server.mvc.handler.HandlerFunctions.http;
54+
55+
/**
56+
* @author jiangyuan
57+
*/
58+
@SpringBootConfiguration
59+
@EnableAutoConfiguration
60+
@LoadBalancerClient(name = "myservice", configuration = MyServiceConf.class)
61+
public class HttpClientApplication {
62+
63+
public static void main(String[] args) {
64+
SpringApplication.run(HttpClientApplication.class, args);
65+
}
66+
67+
@Bean
68+
public HttpComponentsClientHttpRequestFactory httpComponentsClientHttpRequestFactory() {
69+
PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager();
70+
connectionManager.setMaxTotal(2);
71+
connectionManager.setDefaultMaxPerRoute(2);
72+
73+
CloseableHttpClient httpClient = HttpClients.custom()
74+
.setConnectionManager(connectionManager)
75+
.setDefaultRequestConfig(
76+
RequestConfig.custom().setConnectionRequestTimeout(Timeout.of(Duration.ofMillis(3000))).build())
77+
.build();
78+
79+
HttpComponentsClientHttpRequestFactory factory = new HttpComponentsClientHttpRequestFactory(httpClient);
80+
return factory;
81+
}
82+
83+
@Bean
84+
public RouterFunction<ServerResponse> gatewayRouterFunctionsRetry() {
85+
return route("test-retry").GET("/retry", http())
86+
.filter(lb("myservice"))
87+
.filter(prefixPath("/do"))
88+
.filter(retry(3))
89+
.build();
90+
}
91+
92+
@RestController
93+
protected static class RetryController {
94+
95+
Log log = LogFactory.getLog(getClass());
96+
97+
ConcurrentHashMap<String, AtomicInteger> map = new ConcurrentHashMap<>();
98+
99+
@GetMapping("/do/retry")
100+
public ResponseEntity<String> retry(@RequestParam("key") String key,
101+
@RequestParam(name = "count", defaultValue = "3") int count,
102+
@RequestParam(name = "failStatus", required = false) Integer failStatus) {
103+
AtomicInteger num = map.computeIfAbsent(key, s -> new AtomicInteger());
104+
int i = num.incrementAndGet();
105+
log.warn("Retry count: " + i);
106+
String body = String.valueOf(i);
107+
if (i < count) {
108+
HttpStatus httpStatus = HttpStatus.INTERNAL_SERVER_ERROR;
109+
if (failStatus != null) {
110+
httpStatus = HttpStatus.resolve(failStatus);
111+
}
112+
return ResponseEntity.status(httpStatus).header("X-Retry-Count", body).body("temporarily broken");
113+
}
114+
return ResponseEntity.status(HttpStatus.OK).header("X-Retry-Count", body).body(body);
115+
}
116+
117+
}
118+
119+
}
120+
121+
class MyServiceConf {
122+
123+
@Value("${local.server.port}")
124+
private int port = 0;
125+
126+
@Bean
127+
public ServiceInstanceListSupplier staticServiceInstanceListSupplier() {
128+
return ServiceInstanceListSuppliers.from("myservice",
129+
new DefaultServiceInstance("myservice-1", "myservice", "localhost", port, false));
130+
}
131+
132+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
logging:
2+
level:
3+
org.springframework.cloud.gateway: TRACE
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright 2013-2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.cloud.gateway.tests.httpclient;
18+
19+
import org.junit.jupiter.api.Test;
20+
21+
import org.springframework.boot.test.context.SpringBootTest;
22+
import org.springframework.boot.test.web.server.LocalServerPort;
23+
import org.springframework.test.annotation.DirtiesContext;
24+
import org.springframework.test.web.reactive.server.WebTestClient;
25+
26+
/**
27+
* @author jiangyuan
28+
*/
29+
@SpringBootTest(classes = HttpClientApplication.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
30+
@DirtiesContext
31+
public class HttpClientApplicationTests {
32+
33+
@LocalServerPort
34+
private int port;
35+
36+
@Test
37+
public void retryWorks() {
38+
WebTestClient client = WebTestClient.bindToServer().baseUrl("http://localhost:" + port).build();
39+
client.get().uri("/retry?key=get").exchange().expectStatus().isOk().expectBody(String.class).isEqualTo("3");
40+
}
41+
42+
}

spring-cloud-gateway-integration-tests/pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
<module>grpc</module>
2525
<module>http2</module>
2626
<module>mvc-failure-analyzer</module>
27+
<module>httpclient</module>
2728
</modules>
2829

2930
<build>

spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/MvcUtils.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ public abstract class MvcUtils {
6363
*/
6464
public static final String CLIENT_RESPONSE_INPUT_STREAM_ATTR = qualify("cachedClientResponseBody");
6565

66+
/**
67+
* Client response key.
68+
*/
69+
public static final String CLIENT_RESPONSE_ATTR = qualify("cachedClientResponse");
70+
6671
/**
6772
* CircuitBreaker execution exception attribute name.
6873
*/

spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/filter/RetryFilterFunctions.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.springframework.http.HttpMethod;
3434
import org.springframework.http.HttpStatus;
3535
import org.springframework.http.HttpStatusCode;
36+
import org.springframework.http.client.ClientHttpResponse;
3637
import org.springframework.retry.RetryContext;
3738
import org.springframework.retry.RetryPolicy;
3839
import org.springframework.retry.policy.CompositeRetryPolicy;
@@ -75,6 +76,7 @@ public static HandlerFilterFunction<ServerResponse, ServerResponse> retry(RetryC
7576
if (config.isCacheBody()) {
7677
MvcUtils.getOrCacheBody(request);
7778
}
79+
reset(request);
7880
ServerResponse serverResponse = next.handle(request);
7981

8082
if (isRetryableStatusCode(serverResponse.statusCode(), config)
@@ -86,6 +88,14 @@ && isRetryableMethod(request.method(), config)) {
8688
});
8789
}
8890

91+
private static void reset(ServerRequest request) throws IOException {
92+
ClientHttpResponse clientHttpResponse = MvcUtils.getAttribute(request, MvcUtils.CLIENT_RESPONSE_ATTR);
93+
if (clientHttpResponse != null) {
94+
clientHttpResponse.close();
95+
MvcUtils.putAttribute(request, MvcUtils.CLIENT_RESPONSE_ATTR, null);
96+
}
97+
}
98+
8999
private static boolean isRetryableStatusCode(HttpStatusCode httpStatus, RetryConfig config) {
90100
return config.getSeries().stream().anyMatch(series -> HttpStatus.Series.resolve(httpStatus.value()) == series);
91101
}

spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/ClientHttpRequestFactoryProxyExchange.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public ServerResponse exchange(Request request) {
5656
InputStream body = clientHttpResponse.getBody();
5757
// put the body input stream in a request attribute so filters can read it.
5858
MvcUtils.putAttribute(request.getServerRequest(), MvcUtils.CLIENT_RESPONSE_INPUT_STREAM_ATTR, body);
59+
MvcUtils.putAttribute(request.getServerRequest(), MvcUtils.CLIENT_RESPONSE_ATTR, clientHttpResponse);
5960
ServerResponse serverResponse = GatewayServerResponse.status(clientHttpResponse.getStatusCode())
6061
.build((req, httpServletResponse) -> {
6162
try (clientHttpResponse) {

spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/RestClientProxyExchange.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ private ServerResponse doExchange(Request request, ClientHttpResponse clientResp
7272
InputStream body = clientResponse.getBody();
7373
// put the body input stream in a request attribute so filters can read it.
7474
MvcUtils.putAttribute(request.getServerRequest(), MvcUtils.CLIENT_RESPONSE_INPUT_STREAM_ATTR, body);
75+
MvcUtils.putAttribute(request.getServerRequest(), MvcUtils.CLIENT_RESPONSE_ATTR, clientResponse);
7576
ServerResponse serverResponse = GatewayServerResponse.status(clientResponse.getStatusCode())
7677
.build((req, httpServletResponse) -> {
7778
try (clientResponse) {

0 commit comments

Comments
 (0)