Skip to content

Bug fix: connections managers to ensure open connections have socket timeout set based on ConnectionConfig upon lease #724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ok2c merged 1 commit into from
Sep 15, 2025
Merged

Bug fix: connections managers to ensure open connections have socket timeout set based on ConnectionConfig upon lease #724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 53 additions & 9 deletions ...ting/src/test/java/org/apache/hc/client5/testing/async/TestAsyncConnectionManagement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import org.apache.hc.client5.http.async.methods.SimpleRequestProducer;
import org.apache.hc.client5.http.async.methods.SimpleResponseConsumer;
import org.apache.hc.client5.http.config.ConnectionConfig;
import org.apache.hc.client5.http.impl.ConnectionHolder;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManager;
import org.apache.hc.client5.http.impl.nio.PoolingAsyncClientConnectionManagerBuilder;
import org.apache.hc.client5.http.nio.AsyncConnectionEndpoint;
Expand All @@ -45,6 +46,7 @@
import org.apache.hc.client5.testing.extension.async.ServerProtocolLevel;
import org.apache.hc.client5.testing.extension.async.TestAsyncClient;
import org.apache.hc.core5.http.ContentType;
import org.apache.hc.core5.http.HttpConnection;
import org.apache.hc.core5.http.HttpHeaders;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpStatus;
Expand Down Expand Up @@ -89,11 +91,9 @@ protected SimpleHttpResponse handle(
void testReleaseConnection() throws Exception {
final HttpHost target = startServer();

final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create()
.build();
configureClient(builder -> builder.setConnectionManager(connManager));
final TestAsyncClient client = startClient();

final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager();
connManager.setMaxTotal(1);

final HttpRoute route = new HttpRoute(target, null, false);
Expand Down Expand Up @@ -159,11 +159,9 @@ void testReleaseConnection() throws Exception {
void testReleaseConnectionWithTimeLimits() throws Exception {
final HttpHost target = startServer();

final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create()
.build();
configureClient(builder -> builder.setConnectionManager(connManager));
final TestAsyncClient client = startClient();

final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager();
connManager.setMaxTotal(1);

final HttpRoute route = new HttpRoute(target, null, false);
Expand Down Expand Up @@ -218,11 +216,9 @@ void testReleaseConnectionWithTimeLimits() throws Exception {
void testCloseExpiredIdleConnections() throws Exception {
final HttpHost target = startServer();

final PoolingAsyncClientConnectionManager connManager = PoolingAsyncClientConnectionManagerBuilder.create()
.build();
configureClient(builder -> builder.setConnectionManager(connManager));
final TestAsyncClient client = startClient();

final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager();
connManager.setMaxTotal(1);

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

@Test
void testConnectionTimeoutSetting() throws Exception {
final HttpHost target = startServer();

final TestAsyncClient client = startClient();

final Timeout connectionSocketTimeout = Timeout.ofMinutes(5);

final PoolingAsyncClientConnectionManager connManager = client.getConnectionManager();
connManager.setMaxTotal(1);
connManager.setDefaultConnectionConfig(ConnectionConfig.custom()
.setSocketTimeout(connectionSocketTimeout)
.build());

final HttpRoute route = new HttpRoute(target, null, false);

final SimpleHttpRequest request = SimpleRequestBuilder.get()
.setHttpHost(target)
.setPath("/")
.addHeader(HttpHeaders.HOST, target.toHostString())
.build();
final HttpClientContext context = HttpClientContext.create();

final Future<AsyncConnectionEndpoint> endpointFuture1 = connManager.lease("id1", route, null, TIMEOUT, null);
final AsyncConnectionEndpoint endpoint1 = endpointFuture1.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit());

final Future<AsyncConnectionEndpoint> connectFuture1 = connManager.connect(endpoint1, client.getImplementation(), TIMEOUT, null, context, null);
final AsyncConnectionEndpoint openEndpoint1 = connectFuture1.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit());

// Modify socket timeout of the endpoint
endpoint1.setSocketTimeout(Timeout.ofSeconds(30));

final Future<SimpleHttpResponse> responseFuture1 = openEndpoint1.execute("ex-1", SimpleRequestProducer.create(request), SimpleResponseConsumer.create(), null);
final SimpleHttpResponse response1 = responseFuture1.get(TIMEOUT.getDuration(), TIMEOUT.getTimeUnit());
Assertions.assertEquals(HttpStatus.SC_OK, response1.getCode());

connManager.release(endpoint1, null, TimeValue.NEG_ONE_MILLISECOND);

final Future<AsyncConnectionEndpoint> endpointFuture2 = connManager.lease("id2", route, null, TIMEOUT, null);
final AsyncConnectionEndpoint endpoint2 = endpointFuture2.get(LEASE_TIMEOUT.getDuration(), LEASE_TIMEOUT.getTimeUnit());
Assertions.assertTrue(endpoint2.isConnected());

final HttpConnection connection = ((ConnectionHolder) endpoint2).get();
Assertions.assertEquals(connectionSocketTimeout, connection.getSocketTimeout());

connManager.close();
}

}
48 changes: 48 additions & 0 deletions ...t5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionManagement.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

import org.apache.hc.client5.http.HttpRoute;
import org.apache.hc.client5.http.config.ConnectionConfig;
import org.apache.hc.client5.http.impl.ConnectionHolder;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManagerBuilder;
import org.apache.hc.client5.http.io.ConnectionEndpoint;
Expand All @@ -42,6 +43,7 @@
import org.apache.hc.client5.testing.extension.sync.TestClient;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpConnection;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpStatus;
Expand Down Expand Up @@ -326,4 +328,50 @@ void testCloseExpiredTTLConnections() throws Exception {
connManager.close();
}

@Test
void testConnectionTimeoutSetting() throws Exception {
configureServer(bootstrap -> bootstrap
.register("/random/*", new RandomHandler()));
final HttpHost target = startServer();

final Timeout connectionSocketTimeout = Timeout.ofMinutes(5);

final TestClient client = client();
final PoolingHttpClientConnectionManager connManager = client.getConnectionManager();
connManager.setMaxTotal(1);
connManager.setDefaultConnectionConfig(ConnectionConfig.custom()
.setSocketTimeout(connectionSocketTimeout)
.build());

final HttpRoute route = new HttpRoute(target, null, false);
final int rsplen = 8;
final String uri = "/random/" + rsplen;

final ClassicHttpRequest request = new BasicClassicHttpRequest("GET", target, uri);
final HttpClientContext context = HttpClientContext.create();

final LeaseRequest leaseRequest1 = connManager.lease("id1", route, null);
final ConnectionEndpoint endpoint1 = leaseRequest1.get(Timeout.ZERO_MILLISECONDS);

connManager.connect(endpoint1, null, context);

// Modify socket timeout of the endpoint
endpoint1.setSocketTimeout(Timeout.ofSeconds(30));

try (final ClassicHttpResponse response1 = endpoint1.execute("id1", request, exec, context)) {
Assertions.assertEquals(HttpStatus.SC_OK, response1.getCode());
}

connManager.release(endpoint1, null, TimeValue.NEG_ONE_MILLISECOND);

final LeaseRequest leaseRequest2 = connManager.lease("id2", route, null);
final ConnectionEndpoint endpoint2 = leaseRequest2.get(Timeout.ZERO_MILLISECONDS);
Assertions.assertTrue(endpoint2.isConnected());

final HttpConnection connection = ((ConnectionHolder) endpoint2).get();
Assertions.assertEquals(connectionSocketTimeout, connection.getSocketTimeout());

connManager.close();
}

}
3 changes: 3 additions & 0 deletions ...t5/src/main/java/org/apache/hc/client5/http/impl/io/BasicHttpClientConnectionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,9 @@ ManagedHttpClientConnection getConnection(final HttpRoute route, final Object st
this.created = System.currentTimeMillis();
} else {
this.conn.activate();
if (connectionConfig.getSocketTimeout() != null) {
conn.setSocketTimeout(connectionConfig.getSocketTimeout());
}
}
this.leased = true;
if (LOG.isDebugEnabled()) {
Expand Down
3 changes: 3 additions & 0 deletions .../src/main/java/org/apache/hc/client5/http/impl/io/PoolingHttpClientConnectionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,9 @@ public ConnectionEndpoint get(
final ManagedHttpClientConnection conn = poolEntry.getConnection();
if (conn != null) {
conn.activate();
if (connectionConfig.getSocketTimeout() != null) {
conn.setSocketTimeout(connectionConfig.getSocketTimeout());
}
} else {
poolEntry.assignConnection(connFactory.createConnection(null));
}
Expand Down
3 changes: 3 additions & 0 deletions ...rc/main/java/org/apache/hc/client5/http/impl/nio/PoolingAsyncClientConnectionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,9 @@ void leaseCompleted(final PoolEntry<HttpRoute, ManagedAsyncClientConnection> poo
final ManagedAsyncClientConnection connection = poolEntry.getConnection();
if (connection != null) {
connection.activate();
if (connectionConfig.getSocketTimeout() != null) {
connection.setSocketTimeout(connectionConfig.getSocketTimeout());
}
}
if (LOG.isDebugEnabled()) {
LOG.debug("{} endpoint leased {}", id, ConnPoolSupport.formatStats(route, state, pool));
Expand Down