From 22484a18ce3ff7d04e0248e498cfc36487bb24ac Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 12 Nov 2018 19:06:18 -0600 Subject: [PATCH 01/11] Updating jetty-bom to 9.4.13.v20181111 --- jetty-bom/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-bom/pom.xml b/jetty-bom/pom.xml index 086b587dd3f..e7f19ad67c4 100644 --- a/jetty-bom/pom.xml +++ b/jetty-bom/pom.xml @@ -2,7 +2,7 @@ org.eclipse.jetty 4.0.0 jetty-bom - 9.4.13-SNAPSHOT + 9.4.13.v20181111 Jetty :: Bom Jetty BOM artifact http://www.eclipse.org/jetty From 6f34541bfd78ab1218b680baa2d78933a46085d9 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 12 Nov 2018 19:09:10 -0600 Subject: [PATCH 02/11] Bumping up version of jetty-bom to 9.4.14-SNAPSHOT --- jetty-bom/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-bom/pom.xml b/jetty-bom/pom.xml index a1047ddbc53..abb972743ec 100644 --- a/jetty-bom/pom.xml +++ b/jetty-bom/pom.xml @@ -2,7 +2,7 @@ org.eclipse.jetty 4.0.0 jetty-bom - 9.4.13.v20181111 + 9.4.14-SNAPSHOT Jetty :: Bom Jetty BOM artifact http://www.eclipse.org/jetty From 26eecd6f8e0e1765728ceeb5000c7841583d72a7 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 13 Nov 2018 10:32:17 +0100 Subject: [PATCH 03/11] HttpClient Dump Improvements Add or update beans in constructor or setter rather than in doStart Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/client/HttpClient.java | 53 +++++++++------ .../jetty/client/ProtocolHandlers.java | 16 ++++- .../client/AbstractHttpClientServerTest.java | 22 ++++++- .../eclipse/jetty/client/HttpClientTest.java | 64 +++++++++++-------- .../client/HttpConnectionLifecycleTest.java | 10 ++- .../client/ValidatingConnectionPoolTest.java | 18 ++++-- 6 files changed, 126 insertions(+), 57 deletions(-) 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 35ade51eb76..9179a8d30e3 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 @@ -18,6 +18,7 @@ package org.eclipse.jetty.client; +import java.io.IOException; import java.net.CookieManager; import java.net.CookiePolicy; import java.net.CookieStore; @@ -70,6 +71,8 @@ import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; 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.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.ssl.SslContextFactory; @@ -179,12 +182,19 @@ public class HttpClient extends ContainerLifeCycle public HttpClient(HttpClientTransport transport, SslContextFactory sslContextFactory) { this.transport = transport; + transport.setHttpClient(this); + addBean(transport); + if (sslContextFactory == null) { sslContextFactory = new SslContextFactory(false); sslContextFactory.setEndpointIdentificationAlgorithm("HTTPS"); } this.sslContextFactory = sslContextFactory; + addBean(sslContextFactory); + addBean(handlers); + addBean(decoderFactories); + addBean(new DumpableCollection("requestListeners", requestListeners)); } public HttpClientTransport getTransport() @@ -204,38 +214,31 @@ public class HttpClient extends ContainerLifeCycle @Override protected void doStart() throws Exception { - // TODO use #addBean in constructor? - if (sslContextFactory != null) - addBean(sslContextFactory); - if (executor == null) { QueuedThreadPool threadPool = new QueuedThreadPool(); threadPool.setName(name); executor = threadPool; + addBean(executor); } - // TODO use #updateBean in #setExecutor - addBean(executor); - + if (byteBufferPool == null) + { byteBufferPool = new MappedByteBufferPool(2048, - executor instanceof ThreadPool.SizedThreadPool - ? ((ThreadPool.SizedThreadPool)executor).getMaxThreads()/2 - : ProcessorUtils.availableProcessors()*2); - // TODO use #updateBean in #setByteBufferPool? - addBean(byteBufferPool); + executor instanceof ThreadPool.SizedThreadPool + ? ((ThreadPool.SizedThreadPool)executor).getMaxThreads() / 2 + : ProcessorUtils.availableProcessors() * 2); + addBean(byteBufferPool); + } if (scheduler == null) - scheduler = new ScheduledExecutorScheduler(name + "-scheduler", false); - // TODO use #updateBean in #setScheduler? - addBean(scheduler); - - transport.setHttpClient(this); - addBean(transport); + setScheduler(new ScheduledExecutorScheduler(name + "-scheduler", false)); if (resolver == null) + { resolver = new SocketAddressResolver.Async(executor, scheduler, getAddressResolutionTimeout()); - addBean(resolver); + addBean(resolver); + } handlers.put(new ContinueProtocolHandler()); handlers.put(new RedirectProtocolHandler(this)); @@ -649,6 +652,9 @@ public class HttpClient extends ContainerLifeCycle */ public void setByteBufferPool(ByteBufferPool byteBufferPool) { + if (isRunning()) + throw new IllegalStateException(getState()); + updateBean(this.byteBufferPool, byteBufferPool); this.byteBufferPool = byteBufferPool; } @@ -801,7 +807,8 @@ public class HttpClient extends ContainerLifeCycle public void setExecutor(Executor executor) { if (isRunning()) - LOG.warn("setExecutor called when in {} state",getState()); + throw new IllegalStateException(getState()); + updateBean(this.executor, executor); this.executor = executor; } @@ -818,6 +825,9 @@ public class HttpClient extends ContainerLifeCycle */ public void setScheduler(Scheduler scheduler) { + if (isStarted()) + throw new IllegalStateException(getState()); + updateBean(this.scheduler, scheduler); this.scheduler = scheduler; } @@ -834,6 +844,9 @@ public class HttpClient extends ContainerLifeCycle */ public void setSocketAddressResolver(SocketAddressResolver resolver) { + if (isRunning()) + throw new IllegalStateException(getState()); + updateBean(this.resolver, resolver); this.resolver = resolver; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandlers.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandlers.java index 0d552f5ff42..49f972eb605 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandlers.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandlers.java @@ -18,16 +18,18 @@ package org.eclipse.jetty.client; +import java.io.IOException; import java.util.LinkedHashMap; import java.util.Map; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; +import org.eclipse.jetty.util.component.Dumpable; /** *

A container for {@link ProtocolHandler}s accessible from {@link HttpClient#getProtocolHandlers()}.

*/ -public class ProtocolHandlers +public class ProtocolHandlers implements Dumpable { private final Map handlers = new LinkedHashMap<>(); @@ -91,4 +93,16 @@ public class ProtocolHandlers } return null; } + + @Override + public String dump() + { + return Dumpable.dump(this); + } + + @Override + public void dump(Appendable out, String indent) throws IOException + { + Dumpable.dumpObjects(out, indent, this.toString(), handlers); + } } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java index 16858c0c879..00dae76f79e 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.client; import java.nio.file.Path; +import java.util.concurrent.Executor; import java.util.stream.Stream; import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; @@ -30,6 +31,8 @@ import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; +import org.eclipse.jetty.util.thread.Scheduler; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.provider.Arguments; @@ -69,14 +72,27 @@ public abstract class AbstractHttpClientServerTest protected void startClient(final Scenario scenario, HttpClientTransport transport) throws Exception { + if (transport==null) + transport = new HttpClientTransportOverHTTP(1); + QueuedThreadPool clientThreads = new QueuedThreadPool(); clientThreads.setName("client"); - client = new HttpClient(transport, scenario.newSslContextFactory()); - client.setExecutor(clientThreads); - client.setSocketAddressResolver(new SocketAddressResolver.Sync()); + Scheduler scheduler = new ScheduledExecutorScheduler("client-scheduler", false); + client = newHttpClient(scenario, transport, clientThreads, scheduler, null); client.start(); } + public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport, Executor executor, Scheduler scheduler, SocketAddressResolver resolver) + { + HttpClient client = new HttpClient(transport, scenario.newSslContextFactory()); + client.setExecutor(executor); + client.setScheduler(scheduler); + if (resolver==null) + resolver = new SocketAddressResolver.Sync(); + client.setSocketAddressResolver(resolver); + return client; + } + @AfterEach public void disposeClient() throws Exception { 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 9afbf8c32fa..8629b04ef47 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 @@ -54,6 +54,7 @@ import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Exchanger; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; @@ -99,6 +100,7 @@ import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.log.StacklessLogging; +import org.eclipse.jetty.util.thread.Scheduler; import org.hamcrest.Matchers; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; @@ -111,6 +113,42 @@ public class HttpClientTest extends AbstractHttpClientServerTest { public WorkDir testdir; + public boolean asyncResolver = false; + + @Override + public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport, Executor executor, Scheduler scheduler, SocketAddressResolver resolver) + { + if (asyncResolver && resolver==null) + { + resolver = new SocketAddressResolver.Async(executor, scheduler, 5000) + { + @Override + public void resolve(String host, int port, Promise> promise) + { + super.resolve(host, port, new Promise>() + { + @Override + public void succeeded(List result) + { + // Add as first address an invalid address so that we test + // that the connect operation iterates over the addresses. + result.add(0, new InetSocketAddress("idontexist", port)); + promise.succeeded(result); + } + + @Override + public void failed(Throwable x) + { + promise.failed(x); + } + }); + } + }; + } + + return super.newHttpClient(scenario, transport, executor, scheduler, resolver); + } + @ParameterizedTest @ArgumentsSource(ScenarioProvider.class) public void testStoppingClosesConnections(Scenario scenario) throws Exception @@ -880,33 +918,9 @@ public class HttpClientTest extends AbstractHttpClientServerTest @ArgumentsSource(ScenarioProvider.class) public void testConnectHostWithMultipleAddresses(Scenario scenario) throws Exception { + asyncResolver = true; start(scenario, new EmptyServerHandler()); - client.setSocketAddressResolver(new SocketAddressResolver.Async(client.getExecutor(), client.getScheduler(), client.getConnectTimeout()) - { - @Override - public void resolve(String host, int port, Promise> promise) - { - super.resolve(host, port, new Promise>() - { - @Override - public void succeeded(List result) - { - // Add as first address an invalid address so that we test - // that the connect operation iterates over the addresses. - result.add(0, new InetSocketAddress("idontexist", port)); - promise.succeeded(result); - } - - @Override - public void failed(Throwable x) - { - promise.failed(x); - } - }); - } - }); - // If no exceptions the test passes. client.newRequest("localhost", connector.getLocalPort()) .scheme(scenario.getScheme()) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java index 243303f5345..abe3b63cbf5 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java @@ -28,6 +28,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Queue; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import javax.servlet.ServletException; @@ -43,10 +44,12 @@ import org.eclipse.jetty.client.http.HttpDestinationOverHTTP; import org.eclipse.jetty.client.util.ByteBufferContentProvider; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpVersion; -import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.StacklessLogging; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.eclipse.jetty.util.thread.Scheduler; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; import org.junit.jupiter.params.ParameterizedTest; @@ -55,10 +58,11 @@ import org.junit.jupiter.params.provider.ArgumentsSource; public class HttpConnectionLifecycleTest extends AbstractHttpClientServerTest { @Override - public void start(Scenario scenario, Handler handler) throws Exception + public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport, Executor executor, Scheduler scheduler, SocketAddressResolver resolver) { - super.start(scenario, handler); + HttpClient client = super.newHttpClient(scenario, transport, executor, scheduler, resolver); client.setStrictEventOrdering(false); + return client; } @ParameterizedTest diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java index aa9a8381e1c..8797fda72e0 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import java.io.IOException; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import javax.servlet.ServletException; @@ -37,19 +38,26 @@ import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.handler.AbstractHandler; +import org.eclipse.jetty.util.SocketAddressResolver; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.eclipse.jetty.util.thread.Scheduler; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; public class ValidatingConnectionPoolTest extends AbstractHttpClientServerTest { @Override - protected void startClient(final Scenario scenario) throws Exception + public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport, Executor executor, Scheduler scheduler, SocketAddressResolver resolver) { - long timeout = 1000; - HttpClientTransportOverHTTP transport = new HttpClientTransportOverHTTP(1); - transport.setConnectionPoolFactory(destination -> + if (transport==null) + { + long timeout = 1000; + transport = new HttpClientTransportOverHTTP(1); + transport.setConnectionPoolFactory(destination -> new ValidatingConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination, destination.getHttpClient().getScheduler(), timeout)); - startClient(scenario, transport); + } + + return super.newHttpClient(scenario, transport, executor, scheduler, resolver); } @ParameterizedTest From fa38868406f8caee3aea6b19f99842bd030429ba Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 13 Nov 2018 10:37:42 +0100 Subject: [PATCH 04/11] use setters from doStart Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/client/HttpClient.java | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) 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 9179a8d30e3..b8b0af8da08 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 @@ -218,27 +218,20 @@ public class HttpClient extends ContainerLifeCycle { QueuedThreadPool threadPool = new QueuedThreadPool(); threadPool.setName(name); - executor = threadPool; - addBean(executor); + setExecutor(threadPool); } if (byteBufferPool == null) - { - byteBufferPool = new MappedByteBufferPool(2048, + setByteBufferPool(new MappedByteBufferPool(2048, executor instanceof ThreadPool.SizedThreadPool ? ((ThreadPool.SizedThreadPool)executor).getMaxThreads() / 2 - : ProcessorUtils.availableProcessors() * 2); - addBean(byteBufferPool); - } + : ProcessorUtils.availableProcessors() * 2)); if (scheduler == null) setScheduler(new ScheduledExecutorScheduler(name + "-scheduler", false)); if (resolver == null) - { - resolver = new SocketAddressResolver.Async(executor, scheduler, getAddressResolutionTimeout()); - addBean(resolver); - } + setSocketAddressResolver(new SocketAddressResolver.Async(executor, scheduler, getAddressResolutionTimeout())); handlers.put(new ContinueProtocolHandler()); handlers.put(new RedirectProtocolHandler(this)); @@ -652,7 +645,7 @@ public class HttpClient extends ContainerLifeCycle */ public void setByteBufferPool(ByteBufferPool byteBufferPool) { - if (isRunning()) + if (isStarted()) throw new IllegalStateException(getState()); updateBean(this.byteBufferPool, byteBufferPool); this.byteBufferPool = byteBufferPool; @@ -806,7 +799,7 @@ public class HttpClient extends ContainerLifeCycle */ public void setExecutor(Executor executor) { - if (isRunning()) + if (isStarted()) throw new IllegalStateException(getState()); updateBean(this.executor, executor); this.executor = executor; @@ -844,7 +837,7 @@ public class HttpClient extends ContainerLifeCycle */ public void setSocketAddressResolver(SocketAddressResolver resolver) { - if (isRunning()) + if (isStarted()) throw new IllegalStateException(getState()); updateBean(this.resolver, resolver); this.resolver = resolver; From d0afc63cd57b3da24f81b7bbbda694fd72b6614e Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 13 Nov 2018 14:22:19 +0100 Subject: [PATCH 05/11] Fixing HttpClient tests to configure before start --- .../client/ValidatingConnectionPoolTest.java | 8 +++--- .../jetty/util/ssl/SslSelectionDump.java | 9 +++---- .../jetty/http/client/HttpClientLoadTest.java | 27 ++++++++++--------- .../jetty/http/client/TransportScenario.java | 20 +++++++++++--- 4 files changed, 38 insertions(+), 26 deletions(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java index 8797fda72e0..dffcd031a43 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java @@ -39,7 +39,6 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.util.SocketAddressResolver; -import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.Scheduler; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; @@ -50,12 +49,11 @@ public class ValidatingConnectionPoolTest extends AbstractHttpClientServerTest public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport, Executor executor, Scheduler scheduler, SocketAddressResolver resolver) { if (transport==null) - { - long timeout = 1000; transport = new HttpClientTransportOverHTTP(1); - transport.setConnectionPoolFactory(destination -> + + long timeout = 1000; + transport.setConnectionPoolFactory(destination -> new ValidatingConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination, destination.getHttpClient().getScheduler(), timeout)); - } return super.newHttpClient(scenario, transport, executor, scheduler, resolver); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslSelectionDump.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslSelectionDump.java index 53bad5acae2..e19355ed3e7 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslSelectionDump.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslSelectionDump.java @@ -27,10 +27,9 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; -import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.component.Dumpable; -class SslSelectionDump extends ContainerLifeCycle implements Dumpable +class SslSelectionDump implements Dumpable { static class CaptionedList extends ArrayList implements Dumpable { @@ -66,9 +65,7 @@ class SslSelectionDump extends ContainerLifeCycle implements Dumpable String[] includedByConfig) { this.type = type; - addBean(enabled); - addBean(disabled); - + List jvmEnabled = Arrays.asList(enabledByJVM); List excludedPatterns = Arrays.stream(excludedByConfig) .map((entry) -> Pattern.compile(entry)) @@ -165,7 +162,7 @@ class SslSelectionDump extends ContainerLifeCycle implements Dumpable @Override public void dump(Appendable out, String indent) throws IOException { - dumpObjects(out, indent); + Dumpable.dumpObjects(out, indent, this, enabled, disabled); } @Override diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java index 814d9bd011b..471e74aba53 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java @@ -18,6 +18,9 @@ package org.eclipse.jetty.http.client; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertTrue; + import java.io.IOException; import java.io.InterruptedIOException; import java.nio.ByteBuffer; @@ -65,9 +68,6 @@ import org.hamcrest.Matchers; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.jupiter.api.Assertions.assertTrue; - public class HttpClientLoadTest extends AbstractTest { private final Logger logger = Log.getLogger(HttpClientLoadTest.class); @@ -85,11 +85,12 @@ public class HttpClientLoadTest extends AbstractTest + { + client.setByteBufferPool(new LeakTrackingByteBufferPool(new MappedByteBufferPool.Tagged())); + client.setMaxConnectionsPerDestination(32768); + client.setMaxRequestsQueuedPerDestination(1024 * 1024); + }); // At least 25k requests to warmup properly (use -XX:+PrintCompilation to verify JIT activity) int runs = 1; @@ -130,11 +131,13 @@ public class HttpClientLoadTest extends AbstractTest + { + client.setByteBufferPool(new LeakTrackingByteBufferPool(new MappedByteBufferPool.Tagged())); + client.setMaxConnectionsPerDestination(32768); + client.setMaxRequestsQueuedPerDestination(1024 * 1024); + }); - scenario.client.setByteBufferPool(new LeakTrackingByteBufferPool(new MappedByteBufferPool.Tagged())); - scenario.client.setMaxConnectionsPerDestination(32768); - scenario.client.setMaxRequestsQueuedPerDestination(1024 * 1024); int runs = 1; int iterations = 256; diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java index 174c1e51f1c..d66e883ca4f 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/TransportScenario.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.concurrent.BlockingQueue; +import java.util.function.Consumer; import javax.servlet.http.HttpServlet; @@ -270,20 +271,29 @@ public class TransportScenario else setConnectionIdleTimeout(idleTimeout); } - public void start(Handler handler) throws Exception + { + start(handler,null); + } + + public void start(Handler handler, Consumer config) throws Exception { startServer(handler); - startClient(); + startClient(config); } public void start(HttpServlet servlet) throws Exception { startServer(servlet); - startClient(); + startClient(null); } public void startClient() throws Exception + { + startClient(null); + } + + public void startClient(Consumer config) throws Exception { QueuedThreadPool clientThreads = new QueuedThreadPool(); clientThreads.setName("client"); @@ -291,6 +301,10 @@ public class TransportScenario client = newHttpClient(provideClientTransport(transport), sslContextFactory); client.setExecutor(clientThreads); client.setSocketAddressResolver(new SocketAddressResolver.Sync()); + + if (config!=null) + config.accept(client); + client.start(); if (server != null) server.addBean(client); From 1845e6ea48956339ec6aae24ffb20a91903af8a0 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 13 Nov 2018 14:29:12 +0100 Subject: [PATCH 06/11] Issue #39078 Duplicated programmatic listeners. (#3101) Signed-off-by: Jan Bartel --- .../jetty/servlet/ServletContextHandler.java | 6 +- .../servlet/ServletContextHandlerTest.java | 80 +++++++++++++++++++ .../com/acme/initializer/FooInitializer.java | 6 ++ .../java/com/acme/test/AnnotatedListener.java | 3 + .../main/java/com/acme/test/TestListener.java | 3 + 5 files changed, 97 insertions(+), 1 deletion(-) diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java index 21f51b86ad7..92d811c405f 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java @@ -1485,10 +1485,14 @@ public class ServletContextHandler extends ContextHandler throw new IllegalStateException(); if (!_enabled) throw new UnsupportedOperationException(); - super.addListener(t); + + + checkListener(t.getClass()); + ListenerHolder holder = getServletHandler().newListenerHolder(Source.JAVAX_API); holder.setListener(t); getServletHandler().addListener(holder); + addProgrammaticListener(t); } @Override diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java index ecb22584459..9a67a85e5ff 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java @@ -24,15 +24,20 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; import java.io.PrintWriter; +import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.Servlet; +import javax.servlet.ServletContainerInitializer; +import javax.servlet.ServletContext; import javax.servlet.ServletContextEvent; import javax.servlet.ServletContextListener; import javax.servlet.ServletException; @@ -60,6 +65,7 @@ import org.eclipse.jetty.server.handler.gzip.GzipHandler; import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.util.DecoratedObjectFactory; import org.eclipse.jetty.util.Decorator; +import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -72,6 +78,66 @@ public class ServletContextHandlerTest private static final AtomicInteger __testServlets = new AtomicInteger(); + public static class MySCI implements ServletContainerInitializer + { + @Override + public void onStartup(Set> c, ServletContext ctx) throws ServletException + { + //add a programmatic listener + if (ctx.getAttribute("MySCI.startup") != null) + throw new IllegalStateException("MySCI already called"); + ctx.setAttribute("MySCI.startup", Boolean.TRUE); + ctx.addListener(new MyContextListener()); + } + } + + public static class MySCIStarter extends AbstractLifeCycle implements ServletContextHandler.ServletContainerInitializerCaller + { + MySCI _sci = new MySCI(); + ContextHandler.Context _ctx; + + MySCIStarter (ContextHandler.Context ctx) + { + _ctx = ctx; + } + + @Override + protected void doStart() throws Exception + { + super.doStart(); + //call the SCI + try + { + _ctx.setExtendedListenerTypes(true); + _sci.onStartup(Collections.emptySet(), _ctx); + } + finally + { + + } + } + } + + + public static class MyContextListener implements ServletContextListener + { + + @Override + public void contextInitialized(ServletContextEvent sce) + { + assertNull(sce.getServletContext().getAttribute("MyContextListener.contextInitialized")); + sce.getServletContext().setAttribute("MyContextListener.contextInitialized", Boolean.TRUE); + } + + @Override + public void contextDestroyed(ServletContextEvent sce) + { + assertNull(sce.getServletContext().getAttribute("MyContextListener.contextDestroyed")); + sce.getServletContext().setAttribute("MyContextListener.contextDestroyed", Boolean.TRUE); + } + + } + public static class MySessionHandler extends SessionHandler { public void checkSessionListeners (int size) @@ -158,6 +224,20 @@ public class ServletContextHandlerTest sessions.checkSessionListeners(1); } + + @Test + public void testListenerFromSCI() throws Exception + { + ContextHandlerCollection contexts = new ContextHandlerCollection(); + _server.setHandler(contexts); + + ServletContextHandler root = new ServletContextHandler(contexts,"/"); + root.addBean(new MySCIStarter(root.getServletContext()), true); + _server.start(); + assertTrue((Boolean)root.getServletContext().getAttribute("MySCI.startup")); + assertTrue((Boolean)root.getServletContext().getAttribute("MyContextListener.contextInitialized")); + } + @Test public void testFindContainer() throws Exception { diff --git a/tests/test-webapps/test-servlet-spec/test-container-initializer/src/main/java/com/acme/initializer/FooInitializer.java b/tests/test-webapps/test-servlet-spec/test-container-initializer/src/main/java/com/acme/initializer/FooInitializer.java index 11aec5063b3..9ae544d7ce8 100644 --- a/tests/test-webapps/test-servlet-spec/test-container-initializer/src/main/java/com/acme/initializer/FooInitializer.java +++ b/tests/test-webapps/test-servlet-spec/test-container-initializer/src/main/java/com/acme/initializer/FooInitializer.java @@ -63,6 +63,9 @@ public class FooInitializer implements ServletContainerInitializer @Override public void contextInitialized(ServletContextEvent sce) { + if (sce.getServletContext().getAttribute("com.acme.AnnotationTest.listenerTest") != null) + throw new IllegalStateException("FooListener already initialized"); + //Can add a ServletContextListener from a ServletContainerInitializer sce.getServletContext().setAttribute("com.acme.AnnotationTest.listenerTest", Boolean.TRUE); @@ -95,6 +98,9 @@ public class FooInitializer implements ServletContainerInitializer @Override public void onStartup(Set> classes, ServletContext context) { + if (context.getAttribute("com.acme.Foo") != null) + throw new IllegalStateException ("FooInitializer on Startup already called"); + context.setAttribute("com.acme.Foo", new ArrayList(classes)); ServletRegistration.Dynamic reg = context.addServlet("AnnotationTest", "com.acme.AnnotationTest"); context.setAttribute("com.acme.AnnotationTest.complete", (reg == null)); diff --git a/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/AnnotatedListener.java b/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/AnnotatedListener.java index acbeba73d51..884911d54b3 100644 --- a/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/AnnotatedListener.java +++ b/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/AnnotatedListener.java @@ -76,6 +76,9 @@ public class AnnotatedListener implements HttpSessionListener, HttpSessionAttri @Override public void contextInitialized(ServletContextEvent sce) { + if (sce.getServletContext().getAttribute("com.acme.AnnotationTest.sclInjectWebListenerTest") != null) + throw new IllegalStateException("AnnotatedListener already initialized"); + sce.getServletContext().setAttribute("com.acme.AnnotationTest.sclInjectWebListenerTest", Boolean.valueOf(maxAmount!=null)); } diff --git a/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/TestListener.java b/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/TestListener.java index eb48aa0f218..3962fdef9c2 100644 --- a/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/TestListener.java +++ b/tests/test-webapps/test-servlet-spec/test-spec-webapp/src/main/java/com/acme/test/TestListener.java @@ -117,6 +117,9 @@ public class TestListener implements HttpSessionListener, HttpSessionAttributeL @Override public void contextInitialized(ServletContextEvent sce) { + if (sce.getServletContext().getAttribute("com.acme.AnnotationTest.sclInjectTest") != null) + throw new IllegalStateException("TestListener already initialized"); + sce.getServletContext().setAttribute("com.acme.AnnotationTest.sclInjectTest", Boolean.valueOf(maxAmount != null)); //Can't add a ServletContextListener from a ServletContextListener even if it is declared in web.xml From 77095c626debf59cfd1893179c787ea75b08f78a Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 13 Nov 2018 07:37:30 -0600 Subject: [PATCH 07/11] Adding profile to ensure release uses JDK11 (or newer) Signed-off-by: Joakim Erdfelt --- pom.xml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pom.xml b/pom.xml index 916d86c1802..7af1ef9d0b6 100644 --- a/pom.xml +++ b/pom.xml @@ -1143,6 +1143,25 @@ + + maven-enforcer-plugin + + + enforce-java + + enforce + + + + + [11,) + [ERROR] OLD JDK [${java.version}] in use. Jetty Release ${project.version} MUST use JDK 11 or newer + + + + + + true org.apache.maven.plugins From 859004bef4fbbb3b2cf274b21f8247ba8268ff23 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 13 Nov 2018 16:42:12 +0100 Subject: [PATCH 08/11] Issue #3103 Fixed buffer leak Only warn if HttpClient setters are called after start Signed-off-by: Greg Wilkins --- .../main/java/org/eclipse/jetty/client/HttpClient.java | 10 ++++------ .../org/eclipse/jetty/client/ResponseNotifier.java | 6 ------ 2 files changed, 4 insertions(+), 12 deletions(-) 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 b8b0af8da08..5fb4b861360 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 @@ -18,7 +18,6 @@ package org.eclipse.jetty.client; -import java.io.IOException; import java.net.CookieManager; import java.net.CookiePolicy; import java.net.CookieStore; @@ -71,7 +70,6 @@ import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; 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.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -646,7 +644,7 @@ public class HttpClient extends ContainerLifeCycle public void setByteBufferPool(ByteBufferPool byteBufferPool) { if (isStarted()) - throw new IllegalStateException(getState()); + LOG.warn("Calling setByteBufferPool() while started is deprecated"); updateBean(this.byteBufferPool, byteBufferPool); this.byteBufferPool = byteBufferPool; } @@ -800,7 +798,7 @@ public class HttpClient extends ContainerLifeCycle public void setExecutor(Executor executor) { if (isStarted()) - throw new IllegalStateException(getState()); + LOG.warn("Calling setExecutor() while started is deprecated"); updateBean(this.executor, executor); this.executor = executor; } @@ -819,7 +817,7 @@ public class HttpClient extends ContainerLifeCycle public void setScheduler(Scheduler scheduler) { if (isStarted()) - throw new IllegalStateException(getState()); + LOG.warn("Calling setScheduler() while started is deprecated"); updateBean(this.scheduler, scheduler); this.scheduler = scheduler; } @@ -838,7 +836,7 @@ public class HttpClient extends ContainerLifeCycle public void setSocketAddressResolver(SocketAddressResolver resolver) { if (isStarted()) - throw new IllegalStateException(getState()); + LOG.warn("Calling setSocketAddressResolver() while started is deprecated"); updateBean(this.resolver, resolver); this.resolver = resolver; } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ResponseNotifier.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ResponseNotifier.java index e879fd56d88..e4c2ffe3f69 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ResponseNotifier.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ResponseNotifier.java @@ -30,7 +30,6 @@ import org.eclipse.jetty.client.api.Result; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.CountingCallback; -import org.eclipse.jetty.util.Retainable; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -122,13 +121,8 @@ public class ResponseNotifier else { CountingCallback counter = new CountingCallback(callback, contentListeners.size()); - Retainable retainable = callback instanceof Retainable ? (Retainable)callback : null; for (Response.AsyncContentListener listener : contentListeners) - { - if (retainable != null) - retainable.retain(); notifyContent(listener, response, buffer.slice(), counter); - } } } From 78a775396eb9524152f3392afabccffda7c79201 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 13 Nov 2018 16:52:04 +0100 Subject: [PATCH 09/11] Issue #3103 delay setting transport.setHttpClient until late in doStart Do not add DumpableCollection as a bean Signed-off-by: Greg Wilkins --- .../main/java/org/eclipse/jetty/client/HttpClient.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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 5fb4b861360..0ffe40925db 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 @@ -18,6 +18,7 @@ package org.eclipse.jetty.client; +import java.io.IOException; import java.net.CookieManager; import java.net.CookiePolicy; import java.net.CookieStore; @@ -180,7 +181,6 @@ public class HttpClient extends ContainerLifeCycle public HttpClient(HttpClientTransport transport, SslContextFactory sslContextFactory) { this.transport = transport; - transport.setHttpClient(this); addBean(transport); if (sslContextFactory == null) @@ -192,7 +192,12 @@ public class HttpClient extends ContainerLifeCycle addBean(sslContextFactory); addBean(handlers); addBean(decoderFactories); - addBean(new DumpableCollection("requestListeners", requestListeners)); + } + + @Override + public void dump(Appendable out, String indent) throws IOException + { + dumpObjects(out, indent, new DumpableCollection("requestListeners", requestListeners)); } public HttpClientTransport getTransport() @@ -241,6 +246,7 @@ public class HttpClient extends ContainerLifeCycle cookieManager = newCookieManager(); cookieStore = cookieManager.getCookieStore(); + transport.setHttpClient(this); super.doStart(); } From d1f52d153405844e96f36deb27515d9d7d84f222 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 13 Nov 2018 16:57:26 +0100 Subject: [PATCH 10/11] Issue #3103 review results Signed-off-by: Greg Wilkins --- .../main/java/org/eclipse/jetty/client/ProtocolHandlers.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandlers.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandlers.java index 49f972eb605..fac8499cec8 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandlers.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ProtocolHandlers.java @@ -103,6 +103,6 @@ public class ProtocolHandlers implements Dumpable @Override public void dump(Appendable out, String indent) throws IOException { - Dumpable.dumpObjects(out, indent, this.toString(), handlers); + Dumpable.dumpObjects(out, indent, this, handlers); } } From 25914e4776c4cefa9d5846e06fc6247c724ff535 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 13 Nov 2018 17:15:59 +0100 Subject: [PATCH 11/11] Issue #3103 use consumer to configure test HttpClient Signed-off-by: Greg Wilkins --- .../client/AbstractHttpClientServerTest.java | 28 ++++---- .../eclipse/jetty/client/HttpClientTest.java | 67 ++++++++----------- .../client/HttpConnectionLifecycleTest.java | 8 +-- .../client/ValidatingConnectionPoolTest.java | 11 +-- 4 files changed, 46 insertions(+), 68 deletions(-) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java index 00dae76f79e..a8a263772bf 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/AbstractHttpClientServerTest.java @@ -19,7 +19,7 @@ package org.eclipse.jetty.client; import java.nio.file.Path; -import java.util.concurrent.Executor; +import java.util.function.Consumer; import java.util.stream.Stream; import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; @@ -67,30 +67,30 @@ public abstract class AbstractHttpClientServerTest protected void startClient(final Scenario scenario) throws Exception { - startClient(scenario, new HttpClientTransportOverHTTP(1)); + startClient(scenario, null,null); } - protected void startClient(final Scenario scenario, HttpClientTransport transport) throws Exception + protected void startClient(final Scenario scenario, HttpClientTransport transport, Consumer config) throws Exception { if (transport==null) transport = new HttpClientTransportOverHTTP(1); - QueuedThreadPool clientThreads = new QueuedThreadPool(); - clientThreads.setName("client"); + QueuedThreadPool executor = new QueuedThreadPool(); + executor.setName("client"); Scheduler scheduler = new ScheduledExecutorScheduler("client-scheduler", false); - client = newHttpClient(scenario, transport, clientThreads, scheduler, null); + client = newHttpClient(scenario, transport); + client.setExecutor(executor); + client.setScheduler(scheduler); + client.setSocketAddressResolver(new SocketAddressResolver.Sync()); + if (config!=null) + config.accept(client); + client.start(); } - public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport, Executor executor, Scheduler scheduler, SocketAddressResolver resolver) + public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport) { - HttpClient client = new HttpClient(transport, scenario.newSslContextFactory()); - client.setExecutor(executor); - client.setScheduler(scheduler); - if (resolver==null) - resolver = new SocketAddressResolver.Sync(); - client.setSocketAddressResolver(resolver); - return client; + return new HttpClient(transport, scenario.newSslContextFactory()); } @AfterEach 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 8629b04ef47..d3470da5e33 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 @@ -54,7 +54,6 @@ import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Exchanger; import java.util.concurrent.ExecutionException; -import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; @@ -100,7 +99,6 @@ import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.log.StacklessLogging; -import org.eclipse.jetty.util.thread.Scheduler; import org.hamcrest.Matchers; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; @@ -113,41 +111,6 @@ public class HttpClientTest extends AbstractHttpClientServerTest { public WorkDir testdir; - public boolean asyncResolver = false; - - @Override - public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport, Executor executor, Scheduler scheduler, SocketAddressResolver resolver) - { - if (asyncResolver && resolver==null) - { - resolver = new SocketAddressResolver.Async(executor, scheduler, 5000) - { - @Override - public void resolve(String host, int port, Promise> promise) - { - super.resolve(host, port, new Promise>() - { - @Override - public void succeeded(List result) - { - // Add as first address an invalid address so that we test - // that the connect operation iterates over the addresses. - result.add(0, new InetSocketAddress("idontexist", port)); - promise.succeeded(result); - } - - @Override - public void failed(Throwable x) - { - promise.failed(x); - } - }); - } - }; - } - - return super.newHttpClient(scenario, transport, executor, scheduler, resolver); - } @ParameterizedTest @ArgumentsSource(ScenarioProvider.class) @@ -918,8 +881,34 @@ public class HttpClientTest extends AbstractHttpClientServerTest @ArgumentsSource(ScenarioProvider.class) public void testConnectHostWithMultipleAddresses(Scenario scenario) throws Exception { - asyncResolver = true; - start(scenario, new EmptyServerHandler()); + startServer(scenario, new EmptyServerHandler()); + startClient(scenario, null, client -> + { + client.setSocketAddressResolver(new SocketAddressResolver.Async(client.getExecutor(), client.getScheduler(), 5000) + { + @Override + public void resolve(String host, int port, Promise> promise) + { + super.resolve(host, port, new Promise>() + { + @Override + public void succeeded(List result) + { + // Add as first address an invalid address so that we test + // that the connect operation iterates over the addresses. + result.add(0, new InetSocketAddress("idontexist", port)); + promise.succeeded(result); + } + + @Override + public void failed(Throwable x) + { + promise.failed(x); + } + }); + } + }); + }); // If no exceptions the test passes. client.newRequest("localhost", connector.getLocalPort()) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java index abe3b63cbf5..624092493a6 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java @@ -28,7 +28,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Queue; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import javax.servlet.ServletException; @@ -45,11 +44,8 @@ import org.eclipse.jetty.client.util.ByteBufferContentProvider; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.server.handler.AbstractHandler; -import org.eclipse.jetty.util.SocketAddressResolver; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.StacklessLogging; -import org.eclipse.jetty.util.thread.QueuedThreadPool; -import org.eclipse.jetty.util.thread.Scheduler; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; import org.junit.jupiter.params.ParameterizedTest; @@ -58,9 +54,9 @@ import org.junit.jupiter.params.provider.ArgumentsSource; public class HttpConnectionLifecycleTest extends AbstractHttpClientServerTest { @Override - public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport, Executor executor, Scheduler scheduler, SocketAddressResolver resolver) + public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport) { - HttpClient client = super.newHttpClient(scenario, transport, executor, scheduler, resolver); + HttpClient client = super.newHttpClient(scenario, transport); client.setStrictEventOrdering(false); return client; } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java index dffcd031a43..c3f1f8196ba 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ValidatingConnectionPoolTest.java @@ -22,7 +22,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import java.io.IOException; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import javax.servlet.ServletException; @@ -31,31 +30,25 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Request; -import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; import org.eclipse.jetty.client.util.FutureResponseListener; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.handler.AbstractHandler; -import org.eclipse.jetty.util.SocketAddressResolver; -import org.eclipse.jetty.util.thread.Scheduler; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; public class ValidatingConnectionPoolTest extends AbstractHttpClientServerTest { @Override - public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport, Executor executor, Scheduler scheduler, SocketAddressResolver resolver) + public HttpClient newHttpClient(Scenario scenario, HttpClientTransport transport) { - if (transport==null) - transport = new HttpClientTransportOverHTTP(1); - long timeout = 1000; transport.setConnectionPoolFactory(destination -> new ValidatingConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination, destination.getHttpClient().getScheduler(), timeout)); - return super.newHttpClient(scenario, transport, executor, scheduler, resolver); + return super.newHttpClient(scenario, transport); } @ParameterizedTest