diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java index e56d1787d3a..1fbc1eba9e5 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/PoolingHttpDestination.java @@ -210,16 +210,7 @@ public abstract class PoolingHttpDestination extends HttpD if (getHttpExchanges().isEmpty()) { - if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty()) - { - // There is a race condition between this thread removing the destination - // and another thread queueing a request to this same destination. - // If this destination is removed, but the request queued, a new connection - // will be opened, the exchange will be executed and eventually the connection - // will idle timeout and be closed. Meanwhile a new destination will be created - // in HttpClient and will be used for other requests. - getHttpClient().removeDestination(this); - } + tryRemoveIdleDestination(); } else { @@ -237,6 +228,27 @@ public abstract class PoolingHttpDestination extends HttpD connectionPool.close(); } + public void abort(Throwable cause) + { + super.abort(cause); + if (getHttpExchanges().isEmpty()) + tryRemoveIdleDestination(); + } + + private void tryRemoveIdleDestination() + { + if (getHttpClient().isRemoveIdleDestinations() && connectionPool.isEmpty()) + { + // There is a race condition between this thread removing the destination + // and another thread queueing a request to this same destination. + // If this destination is removed, but the request queued, a new connection + // will be opened, the exchange will be executed and eventually the connection + // will idle timeout and be closed. Meanwhile a new destination will be created + // in HttpClient and will be used for other requests. + getHttpClient().removeDestination(this); + } + } + @Override public String toString() { diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java index 9720ce97917..58a32f4f636 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java @@ -280,6 +280,33 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest Assert.assertNotSame(destinationBefore, destinationAfter); } + @Test + public void testDestinationIsRemovedAfterConnectionError() throws Exception + { + String host = "localhost"; + int port = connector.getLocalPort(); + client.setRemoveIdleDestinations(true); + Assert.assertTrue("Destinations of a fresh client must be empty", client.getDestinations().isEmpty()); + + server.stop(); + Request request = client.newRequest(host, port).scheme(this.scheme); + try + { + request.send(); + Assert.fail("Request to a closed port must fail"); + } + catch (Exception expected) + { + } + + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(1); + while (!client.getDestinations().isEmpty() && System.nanoTime() < deadline) + { + Thread.sleep(10); + } + Assert.assertTrue("Destination must be removed after connection error", client.getDestinations().isEmpty()); + } + private Connection timedPoll(Queue connections, long time, TimeUnit unit) throws InterruptedException { long start = System.nanoTime();