Skip to content

Commit cb2ccda

Browse files
ok2cok2c
committed
Bug fix: connections managers to ensure open connections have socket timeout set based on ConnectionConfig upon lease
1 parent 4466cca commit cb2ccda

File tree

5 files changed

+110
-9
lines changed

5 files changed

+110
-9
lines changed

‎httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionManagement.java‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.apache.hc.client5.http.async.methods.SimpleRequestProducer;
3838
import org.apache.hc.client5.http.async.methods.SimpleResponseConsumer;
3939
import org.apache.hc.client5.http.config.ConnectionConfig;
40+
import org.apache.hc.client5.http.impl.ConnectionHolder;
4041
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager;
4142
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
4243
import org.apache.hc.client5.http.nio.AsyncConnectionEndpoint;
@@ -45,6 +46,7 @@
4546
import org.apache.hc.client5.testing.extension.async.ServerProtocolLevel;
4647
import org.apache.hc.client5.testing.extension.async.TestAsyncClient;
4748
import org.apache.hc.core5.http.ContentType;
49+
import org.apache.hc.core5.http.HttpConnection;
4850
import org.apache.hc.core5.http.HttpHeaders;
4951
import org.apache.hc.core5.http.HttpHost;
5052
import org.apache.hc.core5.http.HttpStatus;
@@ -89,11 +91,9 @@ protected SimpleHttpResponse handle(
8991
void testReleaseConnection() throws Exception {
9092
final HttpHost target = startServer();
9193

92-
final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create()
93-
.build();
94-
configureClient(builder -> builder.setConnectionManager(connManager));
9594
final TestAsyncClient client = startClient();
9695

96+
final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager();
9797
connManager.setMaxTotal(1);
9898

9999
final HttpRoute route = new HttpRoute(target, null, false);
@@ -159,11 +159,9 @@ void testReleaseConnection() throws Exception {
159159
void testReleaseConnectionWithTimeLimits() throws Exception {
160160
final HttpHost target = startServer();
161161

162-
final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create()
163-
.build();
164-
configureClient(builder -> builder.setConnectionManager(connManager));
165162
final TestAsyncClient client = startClient();
166163

164+
final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager();
167165
connManager.setMaxTotal(1);
168166

169167
final HttpRoute route = new HttpRoute(target, null, false);
@@ -218,11 +216,9 @@ void testReleaseConnectionWithTimeLimits() throws Exception {
218216
void testCloseExpiredIdleConnections() throws Exception {
219217
final HttpHost target = startServer();
220218

221-
final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create()
222-
.build();
223-
configureClient(builder -> builder.setConnectionManager(connManager));
224219
final TestAsyncClient client = startClient();
225220

221+
final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager();
226222
connManager.setMaxTotal(1);
227223

228224
final HttpRoute route = new HttpRoute(target, null, false);
@@ -312,4 +308,52 @@ void testCloseExpiredTTLConnections() throws Exception {
312308
connManager.close();
313309
}
314310

311+
@Test
312+
void testConnectionTimeoutSetting() throws Exception {
313+
final HttpHost target = startServer();
314+
315+
final TestAsyncClient client = startClient();
316+
317+
final Timeout connectionSocketTimeout = Timeout.ofMinutes(5);
318+
319+
final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager();
320+
connManager.setMaxTotal(1);
321+
connManager.setDefaultConnectionConfig(ConnectionConfig.custom()
322+
.setSocketTimeout(connectionSocketTimeout)
323+
.build());
324+
325+
final HttpRoute route = new HttpRoute(target, null, false);
326+
327+
final SimpleHttpRequest request = SimpleRequestBuilder.get()
328+
.setHttpHost(target)
329+
.setPath("/")
330+
.addHeader(HttpHeaders.HOST, target.toHostString())
331+
.build();
332+
final HttpClientContext context = HttpClientContext.create();
333+
334+
final Future<AsyncConnectionEndpoint> endpointFuture1 = connManager.lease("id1", route, null, TIMEOUT, null);
335+
final AsyncConnectionEndpoint endpoint1 = endpointFuture1.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit());
336+
337+
final Future<AsyncConnectionEndpoint> connectFuture1 = connManager.connect(endpoint1, client.getImplementation(), TIMEOUT, null, context, null);
338+
final AsyncConnectionEndpoint openEndpoint1 = connectFuture1.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit());
339+
340+
// Modify socket timeout of the endpoint
341+
endpoint1.setSocketTimeout(Timeout.ofSeconds(30));
342+
343+
final Future<SimpleHttpResponse> responseFuture1 = openEndpoint1.execute("ex-1", SimpleRequestProducer.create(request), SimpleResponseConsumer.create(), null);
344+
final SimpleHttpResponse response1 = responseFuture1.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit());
345+
Assertions.assertEquals(HttpStatus.SC_OK, response1.getCode());
346+
347+
connManager.release(endpoint1, null, TimeValue.NEG_ONE_MILLISECOND);
348+
349+
final Future<AsyncConnectionEndpoint> endpointFuture2 = connManager.lease("id2", route, null, TIMEOUT, null);
350+
final AsyncConnectionEndpoint endpoint2 = endpointFuture2.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit());
351+
Assertions.assertTrue(endpoint2.isConnected());
352+
353+
final HttpConnection connection = ((ConnectionHolder) endpoint2).get();
354+
Assertions.assertEquals(connectionSocketTimeout, connection.getSocketTimeout());
355+
356+
connManager.close();
357+
}
358+
315359
}

‎httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import org.apache.hc.client5.http.HttpRoute;
3434
import org.apache.hc.client5.http.config.ConnectionConfig;
35+
import org.apache.hc.client5.http.impl.ConnectionHolder;
3536
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
3637
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
3738
import org.apache.hc.client5.http.io.ConnectionEndpoint;
@@ -42,6 +43,7 @@
4243
import org.apache.hc.client5.testing.extension.sync.TestClient;
4344
import org.apache.hc.core5.http.ClassicHttpRequest;
4445
import org.apache.hc.core5.http.ClassicHttpResponse;
46+
import org.apache.hc.core5.http.HttpConnection;
4547
import org.apache.hc.core5.http.HttpException;
4648
import org.apache.hc.core5.http.HttpHost;
4749
import org.apache.hc.core5.http.HttpStatus;
@@ -326,4 +328,50 @@ void testCloseExpiredTTLConnections() throws Exception {
326328
connManager.close();
327329
}
328330

331+
@Test
332+
void testConnectionTimeoutSetting() throws Exception {
333+
configureServer(bootstrap -> bootstrap
334+
.register("/random/*", new RandomHandler()));
335+
final HttpHost target = startServer();
336+
337+
final Timeout connectionSocketTimeout = Timeout.ofMinutes(5);
338+
339+
final TestClient client = client();
340+
final PoolingHttpClientConnectionManager connManager = client.getConnectionManager();
341+
connManager.setMaxTotal(1);
342+
connManager.setDefaultConnectionConfig(ConnectionConfig.custom()
343+
.setSocketTimeout(connectionSocketTimeout)
344+
.build());
345+
346+
final HttpRoute route = new HttpRoute(target, null, false);
347+
final int rsplen = 8;
348+
final String uri = "/random/" + rsplen;
349+
350+
final ClassicHttpRequest request = new BasicClassicHttpRequest("GET", target, uri);
351+
final HttpClientContext context = HttpClientContext.create();
352+
353+
final LeaseRequest leaseRequest1 = connManager.lease("id1", route, null);
354+
final ConnectionEndpoint endpoint1 = leaseRequest1.get(Timeout.ZERO_MILLISECONDS);
355+
356+
connManager.connect(endpoint1, null, context);
357+
358+
// Modify socket timeout of the endpoint
359+
endpoint1.setSocketTimeout(Timeout.ofSeconds(30));
360+
361+
try (final ClassicHttpResponse response1 = endpoint1.execute("id1", request, exec, context)) {
362+
Assertions.assertEquals(HttpStatus.SC_OK, response1.getCode());
363+
}
364+
365+
connManager.release(endpoint1, null, TimeValue.NEG_ONE_MILLISECOND);
366+
367+
final LeaseRequest leaseRequest2 = connManager.lease("id2", route, null);
368+
final ConnectionEndpoint endpoint2 = leaseRequest2.get(Timeout.ZERO_MILLISECONDS);
369+
Assertions.assertTrue(endpoint2.isConnected());
370+
371+
final HttpConnection connection = ((ConnectionHolder) endpoint2).get();
372+
Assertions.assertEquals(connectionSocketTimeout, connection.getSocketTimeout());
373+
374+
connManager.close();
375+
}
376+
329377
}

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,9 @@ ManagedHttpClientConnection getConnection(final HttpRoute route, final Object st
401401
this.created = System.currentTimeMillis();
402402
} else {
403403
this.conn.activate();
404+
if (connectionConfig.getSocketTimeout() != null) {
405+
conn.setSocketTimeout(connectionConfig.getSocketTimeout());
406+
}
404407
}
405408
this.leased = true;
406409
if (LOG.isDebugEnabled()) {

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,9 @@ public ConnectionEndpoint get(
386386
final ManagedHttpClientConnection conn = poolEntry.getConnection();
387387
if (conn != null) {
388388
conn.activate();
389+
if (connectionConfig.getSocketTimeout() != null) {
390+
conn.setSocketTimeout(connectionConfig.getSocketTimeout());
391+
}
389392
} else {
390393
poolEntry.assignConnection(connFactory.createConnection(null));
391394
}

‎httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,9 @@ void leaseCompleted(final PoolEntry<HttpRoute, ManagedAsyncClientConnection> poo
326326
final ManagedAsyncClientConnection connection = poolEntry.getConnection();
327327
if (connection != null) {
328328
connection.activate();
329+
if (connectionConfig.getSocketTimeout() != null) {
330+
connection.setSocketTimeout(connectionConfig.getSocketTimeout());
331+
}
329332
}
330333
if (LOG.isDebugEnabled()) {
331334
LOG.debug("{} endpoint leased {}", id, ConnPoolSupport.formatStats(route, state, pool));

0 commit comments

Comments
 (0)