From 5f9004f00cce8b385e05b037d8210b0d92f741c6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 02:04:33 +0000 Subject: [PATCH 1/9] Bump google-cloud-datastore from 2.10.1 to 2.11.0 Bumps [google-cloud-datastore](https://github.com/googleapis/java-datastore) from 2.10.1 to 2.11.0. - [Release notes](https://github.com/googleapis/java-datastore/releases) - [Changelog](https://github.com/googleapis/java-datastore/blob/main/CHANGELOG.md) - [Commits](https://github.com/googleapis/java-datastore/compare/v2.10.1...v2.11.0) --- updated-dependencies: - dependency-name: com.google.cloud:google-cloud-datastore dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- jetty-gcloud/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-gcloud/pom.xml b/jetty-gcloud/pom.xml index 16338f8024d0..92cf3022cd49 100644 --- a/jetty-gcloud/pom.xml +++ b/jetty-gcloud/pom.xml @@ -13,7 +13,7 @@ Jetty :: GCloud - 2.10.1 + 2.11.0 From b235833f14dbfd74a460adeb466e90ec715dd12a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 02:04:44 +0000 Subject: [PATCH 2/9] Bump maven-jxr-plugin from 3.2.0 to 3.3.0 Bumps [maven-jxr-plugin](https://github.com/apache/maven-jxr) from 3.2.0 to 3.3.0. - [Release notes](https://github.com/apache/maven-jxr/releases) - [Commits](https://github.com/apache/maven-jxr/compare/jxr-3.2.0...jxr-3.3.0) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-jxr-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d471e40b44ea..f95afff247cd 100644 --- a/pom.xml +++ b/pom.xml @@ -154,7 +154,7 @@ 3.3.0 3.2.2 3.4.0 - 3.2.0 + 3.3.0 3.6.4 3.6.4 2.5.3 From 988ccae021906c3b42283c71b586630cc7135d36 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 02:05:03 +0000 Subject: [PATCH 3/9] Bump jboss-threads from 3.1.0.Final to 3.5.0.Final Bumps [jboss-threads](https://github.com/jbossas/jboss-threads) from 3.1.0.Final to 3.5.0.Final. - [Release notes](https://github.com/jbossas/jboss-threads/releases) - [Commits](https://github.com/jbossas/jboss-threads/compare/3.1.0.Final...3.5.0.Final) --- updated-dependencies: - dependency-name: org.jboss.threads:jboss-threads dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d471e40b44ea..937a7199b250 100644 --- a/pom.xml +++ b/pom.xml @@ -82,7 +82,7 @@ 2.2.1.Final 3.5.0.Final 2.1.18.Final - 3.1.0.Final + 3.5.0.Final 1.1 1.0.7 0.12.0 From d5d700451f58d775aceaf55a4f628c7d8d623b85 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 02:05:36 +0000 Subject: [PATCH 4/9] Bump apache.directory.api.version from 2.1.0 to 2.1.2 Bumps `apache.directory.api.version` from 2.1.0 to 2.1.2. Updates `api-ldap-schema-data` from 2.1.0 to 2.1.2 Updates `api-ldap-model` from 2.1.0 to 2.1.2 Updates `api-util` from 2.1.0 to 2.1.2 - [Release notes](https://github.com/apache/directory-ldap-api/releases) - [Commits](https://github.com/apache/directory-ldap-api/compare/2.1.0...2.1.2) Updates `api-asn1-api` from 2.1.0 to 2.1.2 --- updated-dependencies: - dependency-name: org.apache.directory.api:api-ldap-schema-data dependency-type: direct:development update-type: version-update:semver-patch - dependency-name: org.apache.directory.api:api-ldap-model dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.apache.directory.api:api-util dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.apache.directory.api:api-asn1-api dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- jetty-jaas/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-jaas/pom.xml b/jetty-jaas/pom.xml index e4737f4a5736..1679e66878e4 100644 --- a/jetty-jaas/pom.xml +++ b/jetty-jaas/pom.xml @@ -13,7 +13,7 @@ ${project.groupId}.jaas 2.0.0.AM26 - 2.1.0 + 2.1.2 org.eclipse.jetty.jaas.* From 8327e49ea66a0c67fb3fcc78cbd900c6bb17f85f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 02:05:43 +0000 Subject: [PATCH 5/9] Bump wildfly-elytron from 1.20.0.Final to 1.20.1.Final Bumps [wildfly-elytron](https://github.com/wildfly-security/wildfly-elytron) from 1.20.0.Final to 1.20.1.Final. - [Release notes](https://github.com/wildfly-security/wildfly-elytron/releases) - [Commits](https://github.com/wildfly-security/wildfly-elytron/compare/1.20.0.Final...1.20.1.Final) --- updated-dependencies: - dependency-name: org.wildfly.security:wildfly-elytron dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d471e40b44ea..fb76b1fd5d29 100644 --- a/pom.xml +++ b/pom.xml @@ -123,7 +123,7 @@ 1.17.3 3.1.9.Final 1.6.0.Final - 1.20.0.Final + 1.20.1.Final 2.4.7 From aa4777ae4f02ebcee2e3717b96ffc9be2d193a71 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 02:05:55 +0000 Subject: [PATCH 6/9] Bump grpc-core from 1.48.0 to 1.49.0 Bumps [grpc-core](https://github.com/grpc/grpc-java) from 1.48.0 to 1.49.0. - [Release notes](https://github.com/grpc/grpc-java/releases) - [Commits](https://github.com/grpc/grpc-java/compare/v1.48.0...v1.49.0) --- updated-dependencies: - dependency-name: io.grpc:grpc-core dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d471e40b44ea..1cf448874fc5 100644 --- a/pom.xml +++ b/pom.xml @@ -44,7 +44,7 @@ 7.0.5 3.0.2 2.14.0 - 1.48.0 + 1.49.0 2.9.1 31.1-jre 5.1.0 From cd7b0fa67283979aeff1ba719f29da2ad9d039d9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 02:07:09 +0000 Subject: [PATCH 7/9] Bump logback-core from 1.3.0-beta0 to 1.4.0 Bumps [logback-core](https://github.com/qos-ch/logback) from 1.3.0-beta0 to 1.4.0. - [Release notes](https://github.com/qos-ch/logback/releases) - [Commits](https://github.com/qos-ch/logback/compare/v_1.3.0-beta0...v_1.4.0) --- updated-dependencies: - dependency-name: ch.qos.logback:logback-core dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d471e40b44ea..89982b3ae46a 100644 --- a/pom.xml +++ b/pom.xml @@ -103,7 +103,7 @@ 5.9.0 2.0.2 2.18.0 - 1.3.0-beta0 + 1.4.0 3.0.6 10.3.6 1.8.2 From 862eded0e1b8ff869866854352c6f232bcc4a01c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Sep 2022 02:08:37 +0000 Subject: [PATCH 8/9] Bump maven-checkstyle-plugin from 3.1.2 to 3.2.0 Bumps [maven-checkstyle-plugin](https://github.com/apache/maven-checkstyle-plugin) from 3.1.2 to 3.2.0. - [Release notes](https://github.com/apache/maven-checkstyle-plugin/releases) - [Commits](https://github.com/apache/maven-checkstyle-plugin/compare/maven-checkstyle-plugin-3.1.2...maven-checkstyle-plugin-3.2.0) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-checkstyle-plugin dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index d471e40b44ea..7e7f1063f068 100644 --- a/pom.xml +++ b/pom.xml @@ -142,7 +142,7 @@ 3.4.2 5.1.8 3.2.0 - 3.1.2 + 3.2.0 3.10.1 3.3.0 3.0.0 From de13ceff3619777dc1213a4dddc40d79ceedb3dd Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Fri, 26 Aug 2022 11:26:31 +0200 Subject: [PATCH 9/9] Fixes #8493: RemoveIdleDestinations's race condition and improve logging. Signed-off-by: Ludovic Orban --- .../jetty/client/AbstractConnectionPool.java | 8 +- .../org/eclipse/jetty/client/HttpClient.java | 87 ++++++++++-- .../eclipse/jetty/client/HttpDestination.java | 124 +++++++++++++----- .../client/DuplexHttpDestinationTest.java | 16 ++- .../client/HttpClientProxyProtocolTest.java | 1 - .../eclipse/jetty/client/HttpClientTest.java | 36 +++++ .../java/org/eclipse/jetty/util/Pool.java | 18 ++- 7 files changed, 236 insertions(+), 54 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java index e6ca06f27ec5..327c9c7ee0d3 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java @@ -275,7 +275,7 @@ protected void tryCreate(boolean create) { pending.decrementAndGet(); if (LOG.isDebugEnabled()) - LOG.debug("Not creating connection as pool is full, pending: {}", pending); + LOG.debug("Not creating connection as pool {} is full, pending: {}", pool, pending); return; } @@ -517,15 +517,17 @@ public boolean sweep() @Override public String toString() { - return String.format("%s@%x[c=%d/%d/%d,a=%d,i=%d,q=%d]", + return String.format("%s@%x[s=%s,c=%d/%d/%d,a=%d,i=%d,q=%d,p=%s]", getClass().getSimpleName(), hashCode(), + getState(), getPendingConnectionCount(), getConnectionCount(), getMaxConnectionCount(), getActiveConnectionCount(), getIdleConnectionCount(), - destination.getQueuedRequestCount()); + destination.getQueuedRequestCount(), + pool); } private class FutureConnection extends Promise.Completable diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 93c5ea5c3ee2..21faa61bb6f2 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -74,6 +74,7 @@ import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; import org.eclipse.jetty.util.thread.Scheduler; +import org.eclipse.jetty.util.thread.Sweeper; import org.eclipse.jetty.util.thread.ThreadPool; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -142,12 +143,13 @@ public class HttpClient extends ContainerLifeCycle private boolean tcpNoDelay = true; private boolean strictEventOrdering = false; private HttpField encodingField; - private boolean removeIdleDestinations = false; + private long destinationIdleTimeout; private String name = getClass().getSimpleName() + "@" + Integer.toHexString(hashCode()); private HttpCompliance httpCompliance = HttpCompliance.RFC7230; private String defaultRequestContentType = "application/octet-stream"; private boolean useInputDirectByteBuffers = true; private boolean useOutputDirectByteBuffers = true; + private Sweeper destinationSweeper; /** * Creates a HttpClient instance that can perform HTTP/1.1 requests to non-TLS and TLS destinations. @@ -222,7 +224,14 @@ protected void doStart() throws Exception cookieStore = cookieManager.getCookieStore(); transport.setHttpClient(this); + super.doStart(); + + if (getDestinationIdleTimeout() > 0L) + { + destinationSweeper = new Sweeper(scheduler, 1000L); + destinationSweeper.start(); + } } private CookieManager newCookieManager() @@ -233,6 +242,12 @@ private CookieManager newCookieManager() @Override protected void doStop() throws Exception { + if (destinationSweeper != null) + { + destinationSweeper.stop(); + destinationSweeper = null; + } + decoderFactories.clear(); handlers.clear(); @@ -290,6 +305,11 @@ CookieManager getCookieManager() return cookieManager; } + Sweeper getDestinationSweeper() + { + return destinationSweeper; + } + /** * @return the authentication store associated with this instance */ @@ -529,21 +549,28 @@ public Origin createOrigin(HttpRequest request, Origin.Protocol protocol) */ public HttpDestination resolveDestination(Origin origin) { - return destinations.computeIfAbsent(origin, o -> + return destinations.compute(origin, (k, v) -> { - HttpDestination destination = getTransport().newHttpDestination(o); - // Start the destination before it's published to other threads. - addManaged(destination); - if (LOG.isDebugEnabled()) - LOG.debug("Created {}", destination); - return destination; + if (v == null || v.stale()) + { + HttpDestination newDestination = getTransport().newHttpDestination(k); + // Start the destination before it's published to other threads. + addManaged(newDestination); + if (LOG.isDebugEnabled()) + LOG.debug("Created {}; existing: '{}'", newDestination, v); + return newDestination; + } + return v; }); } protected boolean removeDestination(HttpDestination destination) { + boolean removed = destinations.remove(destination.getOrigin(), destination); removeBean(destination); - return destinations.remove(destination.getOrigin(), destination); + if (LOG.isDebugEnabled()) + LOG.debug("Removed {}; result: {}", destination, removed); + return removed; } /** @@ -1010,14 +1037,50 @@ public void setStrictEventOrdering(boolean strictEventOrdering) this.strictEventOrdering = strictEventOrdering; } + /** + * The default value is 0 + * @return the time in ms after which idle destinations are removed + * @see #setDestinationIdleTimeout(long) + */ + @ManagedAttribute("The time in ms after which idle destinations are removed, disabled when zero or negative") + public long getDestinationIdleTimeout() + { + return destinationIdleTimeout; + } + + /** + *

+ * Whether destinations that have no connections (nor active nor idle) and no exchanges + * should be removed after the specified timeout. + *

+ *

+ * If the specified {@code destinationIdleTimeout} is 0 or negative, then the destinations + * are not removed. + *

+ *

+ * Avoids accumulating destinations when applications (e.g. a spider bot or web crawler) + * hit a lot of different destinations that won't be visited again. + *

+ * + * @param destinationIdleTimeout the time in ms after which idle destinations are removed + */ + public void setDestinationIdleTimeout(long destinationIdleTimeout) + { + if (isStarted()) + throw new IllegalStateException(); + this.destinationIdleTimeout = destinationIdleTimeout; + } + /** * @return whether destinations that have no connections should be removed * @see #setRemoveIdleDestinations(boolean) + * @deprecated replaced by {@link #getDestinationIdleTimeout()} */ + @Deprecated @ManagedAttribute("Whether idle destinations are removed") public boolean isRemoveIdleDestinations() { - return removeIdleDestinations; + return destinationIdleTimeout > 0L; } /** @@ -1031,10 +1094,12 @@ public boolean isRemoveIdleDestinations() * * @param removeIdleDestinations whether destinations that have no connections should be removed * @see org.eclipse.jetty.client.DuplexConnectionPool + * @deprecated replaced by {@link #setDestinationIdleTimeout(long)}, calls the latter with a value of 10000 ms. */ + @Deprecated public void setRemoveIdleDestinations(boolean removeIdleDestinations) { - this.removeIdleDestinations = removeIdleDestinations; + setDestinationIdleTimeout(removeIdleDestinations ? 10_000L : 0L); } /** diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java index b27819e68ac0..b09f495fd28e 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java @@ -21,6 +21,7 @@ import java.util.List; import java.util.Queue; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.eclipse.jetty.client.api.Connection; @@ -40,14 +41,16 @@ import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.DumpableCollection; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Scheduler; import org.eclipse.jetty.util.thread.Sweeper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @ManagedObject -public abstract class HttpDestination extends ContainerLifeCycle implements Destination, Closeable, Callback, Dumpable +public abstract class HttpDestination extends ContainerLifeCycle implements Destination, Closeable, Callback, Dumpable, Sweeper.Sweepable { private static final Logger LOG = LoggerFactory.getLogger(HttpDestination.class); @@ -60,7 +63,10 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest private final ClientConnectionFactory connectionFactory; private final HttpField hostField; private final RequestTimeouts requestTimeouts; + private final AutoLock staleLock = new AutoLock(); private ConnectionPool connectionPool; + private boolean stale; + private long activeNanos; public HttpDestination(HttpClient client, Origin origin, boolean intrinsicallySecure) { @@ -104,23 +110,78 @@ public void accept(Connection connection) connectionPool.accept(connection); } + public boolean stale() + { + try (AutoLock l = staleLock.lock()) + { + boolean stale = this.stale; + if (!stale) + this.activeNanos = System.nanoTime(); + if (LOG.isDebugEnabled()) + LOG.debug("Stale check done with result {} on {}", stale, this); + return stale; + } + } + + @Override + public boolean sweep() + { + if (LOG.isDebugEnabled()) + LOG.debug("Sweep check in progress on {}", this); + boolean remove = false; + try (AutoLock l = staleLock.lock()) + { + boolean stale = exchanges.isEmpty() && connectionPool.isEmpty(); + if (!stale) + { + this.activeNanos = System.nanoTime(); + } + else if (isStaleDelayExpired()) + { + this.stale = true; + remove = true; + } + } + if (remove) + { + getHttpClient().removeDestination(this); + LifeCycle.stop(this); + } + if (LOG.isDebugEnabled()) + LOG.debug("Sweep check done with result {} on {}", remove, this); + return remove; + } + + private boolean isStaleDelayExpired() + { + assert staleLock.isHeldByCurrentThread(); + long destinationIdleTimeout = TimeUnit.MILLISECONDS.toNanos(getHttpClient().getDestinationIdleTimeout()); + return System.nanoTime() - activeNanos >= destinationIdleTimeout; + } + @Override protected void doStart() throws Exception { this.connectionPool = newConnectionPool(client); addBean(connectionPool, true); super.doStart(); - Sweeper sweeper = client.getBean(Sweeper.class); - if (sweeper != null && connectionPool instanceof Sweeper.Sweepable) - sweeper.offer((Sweeper.Sweepable)connectionPool); + Sweeper connectionPoolSweeper = client.getBean(Sweeper.class); + if (connectionPoolSweeper != null && connectionPool instanceof Sweeper.Sweepable) + connectionPoolSweeper.offer((Sweeper.Sweepable)connectionPool); + Sweeper destinationSweeper = getHttpClient().getDestinationSweeper(); + if (destinationSweeper != null) + destinationSweeper.offer(this); } @Override protected void doStop() throws Exception { - Sweeper sweeper = client.getBean(Sweeper.class); - if (sweeper != null && connectionPool instanceof Sweeper.Sweepable) - sweeper.remove((Sweeper.Sweepable)connectionPool); + Sweeper destinationSweeper = getHttpClient().getDestinationSweeper(); + if (destinationSweeper != null) + destinationSweeper.remove(this); + Sweeper connectionPoolSweeper = client.getBean(Sweeper.class); + if (connectionPoolSweeper != null && connectionPool instanceof Sweeper.Sweepable) + connectionPoolSweeper.remove((Sweeper.Sweepable)connectionPool); super.doStop(); removeBean(connectionPool); } @@ -449,11 +510,7 @@ public boolean remove(Connection connection) { boolean removed = connectionPool.remove(connection); - if (getHttpExchanges().isEmpty()) - { - tryRemoveIdleDestination(); - } - else if (removed) + if (removed) { // Process queued requests that may be waiting. // We may create a connection that is not @@ -478,22 +535,6 @@ public void abort(Throwable cause) { exchange.getRequest().abort(cause); } - if (exchanges.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 @@ -507,16 +548,39 @@ public String asString() return getOrigin().asString(); } + @ManagedAttribute("For how long this destination has been idle in ms") + public long getIdle() + { + if (getHttpClient().getDestinationIdleTimeout() <= 0L) + return -1; + try (AutoLock l = staleLock.lock()) + { + return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - activeNanos); + } + } + + @ManagedAttribute("Whether this destinations is stale") + public boolean isStale() + { + try (AutoLock l = staleLock.lock()) + { + return this.stale; + } + } + @Override public String toString() { - return String.format("%s[%s]@%x%s,queue=%d,pool=%s", + return String.format("%s[%s]@%x%s,state=%s,queue=%d,pool=%s,stale=%b,idle=%d", HttpDestination.class.getSimpleName(), getOrigin(), hashCode(), proxy == null ? "" : "(via " + proxy + ")", + getState(), getQueuedRequestCount(), - getConnectionPool()); + getConnectionPool(), + isStale(), + getIdle()); } @FunctionalInterface diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/DuplexHttpDestinationTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/DuplexHttpDestinationTest.java index 2dcdccf3e883..850731266197 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/DuplexHttpDestinationTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/DuplexHttpDestinationTest.java @@ -19,6 +19,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.Supplier; +import org.awaitility.Awaitility; import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Destination; @@ -291,6 +292,9 @@ public void testRequestFailedIfMaxRequestsQueuedPerDestinationExceeded(Scenario public void testDestinationIsRemoved(Scenario scenario) throws Exception { start(scenario, new EmptyServerHandler()); + client.stop(); + client.setDestinationIdleTimeout(1000); + client.start(); String host = "localhost"; int port = connector.getLocalPort(); @@ -305,7 +309,7 @@ public void testDestinationIsRemoved(Scenario scenario) throws Exception Destination destinationAfter = client.resolveDestination(request); assertSame(destinationBefore, destinationAfter); - client.setRemoveIdleDestinations(true); + Awaitility.await().atMost(5, TimeUnit.SECONDS).until(() -> client.getDestinations().isEmpty()); request = client.newRequest(host, port) .scheme(scenario.getScheme()) @@ -323,21 +327,19 @@ public void testDestinationIsRemoved(Scenario scenario) throws Exception public void testDestinationIsRemovedAfterConnectionError(Scenario scenario) throws Exception { start(scenario, new EmptyServerHandler()); + client.stop(); + client.setDestinationIdleTimeout(1000); + client.start(); String host = "localhost"; int port = connector.getLocalPort(); - client.setRemoveIdleDestinations(true); assertTrue(client.getDestinations().isEmpty(), "Destinations of a fresh client must be empty"); server.stop(); Request request = client.newRequest(host, port).scheme(scenario.getScheme()); assertThrows(Exception.class, request::send); - long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(1); - while (!client.getDestinations().isEmpty() && System.nanoTime() < deadline) - { - Thread.sleep(10); - } + Awaitility.await().atMost(5, TimeUnit.SECONDS).until(() -> client.getDestinations().isEmpty()); assertTrue(client.getDestinations().isEmpty(), "Destination must be removed after connection error"); } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java index a54353ec8de8..9c882e739484 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientProxyProtocolTest.java @@ -70,7 +70,6 @@ private void startClient() throws Exception clientThreads.setName("client"); client = new HttpClient(); client.setExecutor(clientThreads); - client.setRemoveIdleDestinations(false); client.start(); } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index c755dcc3b242..b7ef4e127258 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -94,6 +94,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.instanceOf; @@ -528,6 +529,41 @@ public void handle(String target, org.eclipse.jetty.server.Request baseRequest, } } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testRetryWithDestinationIdleTimeoutEnabled(Scenario scenario) throws Exception + { + start(scenario, new EmptyServerHandler()); + client.stop(); + client.setDestinationIdleTimeout(1000); + client.setIdleTimeout(1000); + client.setMaxConnectionsPerDestination(1); + client.start(); + + try (StacklessLogging ignored = new StacklessLogging(org.eclipse.jetty.server.HttpChannel.class)) + { + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .path("/one") + .send(); + + int idleTimeout = 100; + Thread.sleep(idleTimeout * 2); + + // After serving a request over a connection that hasn't timed out, serving a second + // request with a shorter idle timeout will make the connection timeout immediately + // after being taken out of the pool. This triggers the retry mechanism. + client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .path("/two") + .idleTimeout(idleTimeout, TimeUnit.MILLISECONDS) + .send(); + } + + // Wait for the sweeper to remove the idle HttpDestination. + await().atMost(5, TimeUnit.SECONDS).until(() -> client.getDestinations().isEmpty()); + } + @ParameterizedTest @ArgumentsSource(ScenarioProvider.class) public void testExchangeIsCompleteOnlyWhenBothRequestAndResponseAreComplete(Scenario scenario) throws Exception diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java index fa86d040e16f..bed066d38e89 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java @@ -127,7 +127,7 @@ public Pool(StrategyType strategyType, int maxEntries) public Pool(StrategyType strategyType, int maxEntries, boolean cache) { this.maxEntries = maxEntries; - this.strategyType = strategyType; + this.strategyType = Objects.requireNonNull(strategyType); this.cache = cache ? new ThreadLocal<>() : null; this.nextIndex = strategyType == StrategyType.ROUND_ROBIN ? new AtomicInteger() : null; } @@ -336,14 +336,25 @@ public Entry reserve() try (AutoLock l = lock.lock()) { if (closed) + { + if (LOGGER.isDebugEnabled()) + LOGGER.debug("{} is closed, returning null reserved entry", this); return null; + } // If we have no space - if (maxEntries > 0 && entries.size() >= maxEntries) + int entriesSize = entries.size(); + if (maxEntries > 0 && entriesSize >= maxEntries) + { + if (LOGGER.isDebugEnabled()) + LOGGER.debug("{} has no space: {} >= {}, returning null reserved entry", this, entriesSize, maxEntries); return null; + } Entry entry = newEntry(); entries.add(entry); + if (LOGGER.isDebugEnabled()) + LOGGER.debug("{} returning new reserved entry {}", this, entry); return entry; } } @@ -514,6 +525,9 @@ public boolean isClosed() @Override public void close() { + if (LOGGER.isDebugEnabled()) + LOGGER.debug("Closing {}", this); + List copy; try (AutoLock l = lock.lock()) {