From a248d38f568116c293eeb2b1f5e998e41bcc0fe0 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 27 Sep 2017 15:36:53 +1000 Subject: [PATCH 01/16] Issue #1854 consistent exception handling for Request parameters --- .../org/eclipse/jetty/server/HttpChannel.java | 3 +- .../org/eclipse/jetty/server/Request.java | 14 ++++---- .../org/eclipse/jetty/server/RequestTest.java | 35 +++++++++++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index a081e80a066..a4f996876d0 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -23,6 +23,7 @@ import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.util.List; import java.util.concurrent.Executor; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Supplier; @@ -522,7 +523,7 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor if (LOG.isDebugEnabled()) LOG.debug(_request.getRequestURI(), failure); } - else if (failure instanceof BadMessageException) + else if (failure instanceof BadMessageException | failure instanceof IOException | failure instanceof TimeoutException) { if (LOG.isDebugEnabled()) LOG.debug(_request.getRequestURI(), failure); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 68ced195dd7..27120d5ebcb 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -75,6 +75,7 @@ import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandler.Context; import org.eclipse.jetty.server.session.Session; @@ -126,6 +127,9 @@ import org.eclipse.jetty.util.log.Logger; * {@link ContextHandler#getMaxFormContentSize()} or if there is no context then the "org.eclipse.jetty.server.Request.maxFormContentSize" {@link Server} * attribute. The number of parameters keys is limited by {@link ContextHandler#getMaxFormKeys()} or if there is no context then the * "org.eclipse.jetty.server.Request.maxFormKeys" {@link Server} attribute. + *

+ *

If IOExceptions or timeouts occur while reading form parameters, these are thrown as unchecked Exceptions: ether {@link RuntimeIOException}, + * {@link BadMessageException} or {@link RuntimeException} as appropriate.

*/ public class Request implements HttpServletRequest { @@ -526,10 +530,8 @@ public class Request implements HttpServletRequest } catch (IOException e) { - if (LOG.isDebugEnabled()) - LOG.warn(e); - else - LOG.warn(e.toString()); + LOG.debug(e); + throw new RuntimeIOException(e); } } @@ -542,8 +544,8 @@ public class Request implements HttpServletRequest } catch (IOException | ServletException e) { - LOG.warn(e); - throw new RuntimeException(e); + LOG.debug(e); + throw new RuntimeIOException(e); } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 5277241ec42..4494810a250 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -68,6 +68,7 @@ import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.toolchain.test.AdvancedRunner; import org.eclipse.jetty.toolchain.test.FS; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.MultiPartInputStreamParser; import org.eclipse.jetty.util.log.Log; @@ -100,6 +101,7 @@ public class RequestTest http.getHttpConfiguration().addCustomizer(new ForwardedRequestCustomizer()); _connector = new LocalConnector(_server,http); _server.addConnector(_connector); + _connector.setIdleTimeout(500); _handler = new RequestHandler(); _server.setHandler(_handler); @@ -177,6 +179,39 @@ public class RequestTest assertThat("Responses", responses, startsWith("HTTP/1.1 400")); } + + @Test + public void testParamExtraction_Timeout() throws Exception + { + _handler._checker = new RequestTester() + { + @Override + public boolean check(HttpServletRequest request,HttpServletResponse response) + { + Map map = request.getParameterMap(); + // should have thrown a BadMessageException + return false; + } + }; + + //Send a request with query string with illegal hex code to cause + //an exception parsing the params + String request="POST / HTTP/1.1\r\n"+ + "Host: whatever\r\n"+ + "Content-Type: "+MimeTypes.Type.FORM_ENCODED.asString()+"\n"+ + "Connection: close\n"+ + "Content-Length: 100\n"+ + "\n"+ + "name=value"; + + LocalEndPoint endp = _connector.connect(); + endp.addInput(request); + + String response = BufferUtil.toString(endp.waitForResponse(false, 1, TimeUnit.SECONDS)); + assertThat("Responses", response, startsWith("HTTP/1.1 500")); + } + + @Test public void testEmptyHeaders() throws Exception { From 55b0f102b0363662613897e70e5735a261e4ecd6 Mon Sep 17 00:00:00 2001 From: WalkerWatch Date: Wed, 27 Sep 2017 14:37:39 -0400 Subject: [PATCH 02/16] Adding authorization examples for #1850. --- .../configuring/security/authentication.adoc | 94 ++++++++++++++++--- 1 file changed, 83 insertions(+), 11 deletions(-) diff --git a/jetty-documentation/src/main/asciidoc/configuring/security/authentication.adoc b/jetty-documentation/src/main/asciidoc/configuring/security/authentication.adoc index 5e27fd9cb7f..c03b18258ae 100644 --- a/jetty-documentation/src/main/asciidoc/configuring/security/authentication.adoc +++ b/jetty-documentation/src/main/asciidoc/configuring/security/authentication.adoc @@ -362,28 +362,100 @@ You define a `JDBCLoginService` with the name of the realm and the location of t [source, xml, subs="{sub-order}"] ---- - Test JDBC Realm etc/jdbcRealm.properties - ---- ==== Authorization -As far as the http://jcp.org/aboutJava/communityprocess/final/jsr340/[Servlet Specification] is concerned, authorization is based on roles. -As we have seen, a LoginService associates a user with a set of roles. -When a user requests a resource that is access protected, the LoginService will be asked to authenticate the user if they are not already, and then asked to confirm if that user possesses one of the roles permitted access to the resource. +As far as the https://jcp.org/en/jsr/detail?id=340[Servlet Specification] is concerned, authorization is based on roles. +As we have seen, a `LoginService` associates a user with a set of roles. +When a user requests a resource that is access protected, the `LoginService` will be asked to authenticate the user if they are not already, and then asked to confirm if that user possesses one of the roles permitted access to the resource. Until Servlet 3.1, role-based authorization could define: -* Access granted to a set of named roles -* Access totally forbidden, regardless of role -* Access granted to a user in any of the roles defined in the effective web.xml. -This is indicated by the special value of `*` for the `` of a `` in the `` +* Access granted to a set of named roles: -With the advent of Servlet 3.1, there is now another authorization: +[source, xml, subs="{sub-order}"] +---- + + + Foo Admin Data + /foo/admin/* + + + admin + manager + + +---- + +* Access totally forbidden, regardless of role: + +[source, xml, subs="{sub-order}"] +---- + + + Foo Protected Data + /foo/protected/* + + + + +---- +* Access granted to a user in any of the roles defined in the effective `web.xml`. +This is indicated by the special value of `*` for the `` of a `` in the ``: + +[source, xml, subs="{sub-order}"] +---- + + + Foo Role Data + /foo/role/* + + + * + + +---- + +Servlet 3.1 introduced an additional authorization: * Access granted to any user who is authenticated, regardless of roles. -This is indicated by the special value of `**` for the `` of a `` in the `` +This is indicated by the special value of `**` for the `` of a `` in the ``: + +[source, xml, subs="{sub-order}"] +---- + + + Foo Authenticated Data + /foo/authenticated/* + + + ** + + +---- + +Additionally, when configuring your security constraints you can protect various HTTP methods as well, such as `PUT`, `GET`, `POST`, `HEAD` or `DELETE`. +This is done by adding the method you want to protect as a `` in the ``. +You can then define roles that should be able to perform these protected methods in an ``: + +[source, xml, subs="{sub-order}"] +---- + + + Foo Authenticated Data + /foo/authenticated/* + DELETE + POST + + + admin + + +---- + +In the above example, only users with an `admin` role will be able to perform `DELETE` or `POST` methods. From 7d98cbb8703d23a30ce830f24f8cfd0fce9fc08a Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 28 Sep 2017 07:35:05 +1000 Subject: [PATCH 03/16] Issue #1851 Improve insufficient thread warnings/errors Squashed commit of the following: commit 1d9e8e4b4d53898cb6435f67529347bd2ba82cf0 Merge: 7280594 55b0f10 Author: Greg Wilkins Date: Thu Sep 28 07:20:37 2017 +1000 Merge branch 'jetty-9.4.x' into jetty-9.4.x-1851-ThreadBudget commit 7280594a0058538b603ad35625713a79830e9b93 Author: Greg Wilkins Date: Wed Sep 27 22:48:58 2017 +1000 fixed headers commit f962f18e5b098ae40846ee3832736ee4650aed84 Author: Greg Wilkins Date: Wed Sep 27 18:12:33 2017 +1000 Issue #1851 added reset commit a63894de284c8d8dc5ed031f1f6e0fccaf6c7715 Author: Greg Wilkins Date: Wed Sep 27 18:08:53 2017 +1000 Issue #1851 improved test commit 8bcc460dc63273165305a7adcb88e991e30de4b7 Author: Greg Wilkins Date: Wed Sep 27 18:03:47 2017 +1000 Issue #1851 Improve insufficient thread warnings/errors Refactor approach to use Leases, to handle multiple executors commit fe4be5f56594f342ab5c2e6c886397d9b4fe9c14 Merge: abc5eac a248d38 Author: Greg Wilkins Date: Wed Sep 27 15:37:56 2017 +1000 Merge branch 'jetty-9.4.x' into jetty-9.4.x-1851-ThreadBudget commit abc5eac2b73d306a91b28ef4db778455095a5bdb Author: Greg Wilkins Date: Wed Sep 27 12:20:03 2017 +1000 Issue #1851 Improve insufficient thread warnings/errors Created a ThreadBudget class that can be used to warn/error for registered and unregistered allocations of threads. The server on doStart does an unregistered check of all its components that implement the Allocation interface. The client will register itself as an Allocation if a shared Executor is used. --- .../jetty/embedded/ManyConnectors.java | 9 +- .../org/eclipse/jetty/client/HttpClient.java | 1 + .../org/eclipse/jetty/io/SelectorManager.java | 5 +- .../jetty/server/AbstractConnector.java | 7 + .../java/org/eclipse/jetty/server/Server.java | 34 +--- .../eclipse/jetty/server/ServerConnector.java | 6 + .../InsufficientThreadsDetectionTest.java | 70 ++++--- .../jetty/util/component/Container.java | 19 ++ .../util/component/ContainerLifeCycle.java | 50 +++++ .../jetty/util/thread/QueuedThreadPool.java | 24 ++- .../util/thread/ReservedThreadExecutor.java | 59 ++++-- .../jetty/util/thread/ThreadBudget.java | 178 ++++++++++++++++++ .../eclipse/jetty/util/thread/ThreadPool.java | 13 +- .../component/ContainerLifeCycleTest.java | 32 ++++ .../jetty/http/client/HttpClientTest.java | 1 + 15 files changed, 430 insertions(+), 78 deletions(-) create mode 100644 jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadBudget.java diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyConnectors.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyConnectors.java index 49016135da9..798434a74a7 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyConnectors.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyConnectors.java @@ -32,6 +32,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.QueuedThreadPool; /** * A Jetty server with multiple connectors. @@ -46,12 +47,14 @@ public class ManyConnectors // probably be a direct path to your own keystore. String jettyDistKeystore = "../../jetty-distribution/target/distribution/demo-base/etc/keystore"; - String keystorePath = System.getProperty( - "example.keystore", jettyDistKeystore); + String keystorePath = System.getProperty("example.keystore", jettyDistKeystore); File keystoreFile = new File(keystorePath); if (!keystoreFile.exists()) { - throw new FileNotFoundException(keystoreFile.getAbsolutePath()); + keystorePath = "jetty-distribution/target/distribution/demo-base/etc/keystore"; + keystoreFile = new File(keystorePath); + if (!keystoreFile.exists()) + throw new FileNotFoundException(keystoreFile.getAbsolutePath()); } // Create a basic jetty server object without declaring the port. Since 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 023da71bfd9..3deae692ce1 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 @@ -73,6 +73,7 @@ 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.eclipse.jetty.util.thread.ThreadBudget; import org.eclipse.jetty.util.thread.ThreadPool; /** diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index 32337188ca8..467920df9bf 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.ReservedThreadExecutor; import org.eclipse.jetty.util.thread.Scheduler; +import org.eclipse.jetty.util.thread.ThreadBudget; import org.eclipse.jetty.util.thread.ThreadPool; import org.eclipse.jetty.util.thread.strategy.EatWhatYouKill; @@ -295,13 +296,15 @@ public abstract class SelectorManager extends ContainerLifeCycle implements Dump @Override protected void doStart() throws Exception { - addBean(new ReservedThreadExecutor(getExecutor(),_reservedThreads),true); + addBean(new ReservedThreadExecutor(getExecutor(),_reservedThreads,this),true); + for (int i = 0; i < _selectors.length; i++) { ManagedSelector selector = newSelector(i); _selectors[i] = selector; addBean(selector); } + super.doStart(); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java index fe0e960b43f..e6c53e2d40a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java @@ -53,6 +53,8 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.Locker; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; import org.eclipse.jetty.util.thread.Scheduler; +import org.eclipse.jetty.util.thread.ThreadBudget; +import org.eclipse.jetty.util.thread.ThreadPool; /** *

An abstract implementation of {@link Connector} that provides a {@link ConnectionFactory} mechanism @@ -158,6 +160,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co private String _name; private int _acceptorPriorityDelta=-2; private boolean _accepting = true; + private ThreadBudget.Lease lease; /** @@ -273,6 +276,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co throw new IllegalStateException("No protocol factory for SSL next protocol: '" + next + "' in " + this); } + lease = ThreadBudget.leaseFrom(getExecutor(),this,_acceptors.length); super.doStart(); _stopping=new CountDownLatch(_acceptors.length); @@ -308,6 +312,9 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co @Override protected void doStop() throws Exception { + if (lease!=null) + lease.close(); + // Tell the acceptors we are stopping interruptAcceptors(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 9e39f3d22a4..f476b58b9fd 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -24,12 +24,12 @@ import java.net.InetSocketAddress; import java.net.URI; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.Enumeration; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.Executor; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -66,6 +66,7 @@ import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.Locker; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.ShutdownThread; +import org.eclipse.jetty.util.thread.ThreadBudget; import org.eclipse.jetty.util.thread.ThreadPool; import org.eclipse.jetty.util.thread.ThreadPool.SizedThreadPool; @@ -379,37 +380,6 @@ public class Server extends HandlerWrapper implements Attributes HttpGenerator.setJettyVersion(HttpConfiguration.SERVER_VERSION); - // Check that the thread pool size is enough. - SizedThreadPool pool = getBean(SizedThreadPool.class); - int max=pool==null?-1:pool.getMaxThreads(); - int selectors=0; - int acceptors=0; - - for (Connector connector : _connectors) - { - if (connector instanceof AbstractConnector) - { - AbstractConnector abstractConnector = (AbstractConnector)connector; - Executor connectorExecutor = connector.getExecutor(); - - if (connectorExecutor != pool) - { - // Do not count the selectors and acceptors from this connector at - // the server level, because the connector uses a dedicated executor. - continue; - } - - acceptors += abstractConnector.getAcceptors(); - - if (connector instanceof ServerConnector) - selectors += ((ServerConnector)connector).getSelectorManager().getSelectorCount(); - } - } - - int needed=1+selectors+acceptors; - if (max>0 && needed>max) - throw new IllegalStateException(String.format("Insufficient threads: max=%d < needed(acceptors=%d + selectors=%d + request=1)",max,acceptors,selectors)); - MultiException mex=new MultiException(); try { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java index 1a07288aa2c..72b1c26175e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ServerConnector.java @@ -555,5 +555,11 @@ public class ServerConnector extends AbstractNetworkConnector onEndPointClosed(endpoint); super.endPointClosed(endpoint); } + + @Override + public String toString() + { + return String.format("SelectorManager@%s",ServerConnector.this); + } } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java index 7b368fc0b8d..c4e53c9bdf3 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java @@ -19,9 +19,12 @@ package org.eclipse.jetty.server; import org.eclipse.jetty.toolchain.test.AdvancedRunner; +import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.eclipse.jetty.util.thread.ThreadBudget; import org.eclipse.jetty.util.thread.ThreadPool; import org.junit.After; +import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -37,21 +40,29 @@ public class InsufficientThreadsDetectionTest _server.stop(); } - @Test(expected = IllegalStateException.class) + @Test() public void testConnectorUsesServerExecutorWithNotEnoughThreads() throws Exception { - // server has 3 threads in the executor - _server = new Server(new QueuedThreadPool(3)); + try + { + // server has 3 threads in the executor + _server = new Server(new QueuedThreadPool(3)); - // connector will use executor from server because connectorPool is null - ThreadPool connectorPool = null; - // connector requires 7 threads(2 + 4 + 1) - ServerConnector connector = new ServerConnector(_server, connectorPool, null, null, 2, 4, new HttpConnectionFactory()); - connector.setPort(0); - _server.addConnector(connector); + // connector will use executor from server because connectorPool is null + ThreadPool connectorPool = null; + // connector requires 7 threads(2 + 4 + 1) + ServerConnector connector = new ServerConnector(_server, connectorPool, null, null, 2, 4, new HttpConnectionFactory()); + connector.setPort(0); + _server.addConnector(connector); - // should throw IllegalStateException because there are no required threads in server pool - _server.start(); + // should throw IllegalStateException because there are no required threads in server pool + _server.start(); + Assert.fail(); + } + catch(IllegalStateException e) + { + Log.getLogger(ThreadBudget.class).warn(e.toString()); + } } @Test @@ -71,20 +82,35 @@ public class InsufficientThreadsDetectionTest _server.start(); } - @Test // Github issue #586 - public void testCaseForMultipleConnectors() throws Exception { - // server has 4 threads in the executor - _server = new Server(new QueuedThreadPool(4)); + // Github issue #586 - // first connector consumes all 4 threads from server pool - _server.addConnector(new ServerConnector(_server, null, null, null, 1, 1, new HttpConnectionFactory())); + @Test + public void testCaseForMultipleConnectors() throws Exception + { + try + { + // server has 4 threads in the executor + _server = new Server(new QueuedThreadPool(4)); - // second connect also require 4 threads but uses own executor, so its threads should not be counted - final QueuedThreadPool connectorPool = new QueuedThreadPool(4, 4); - _server.addConnector(new ServerConnector(_server, connectorPool, null, null, 1, 1, new HttpConnectionFactory())); + // first connector consumes 3 threads from server pool + _server.addConnector(new ServerConnector(_server, null, null, null, 1, 1, new HttpConnectionFactory())); - // should not throw exception because limit was not overflown - _server.start(); + // second connect also require 4 threads but uses own executor, so its threads should not be counted + final QueuedThreadPool connectorPool = new QueuedThreadPool(4, 4); + _server.addConnector(new ServerConnector(_server, connectorPool, null, null, 1, 1, new HttpConnectionFactory())); + + // first connector consumes 3 threads from server pool + _server.addConnector(new ServerConnector(_server, null, null, null, 1, 1, new HttpConnectionFactory())); + + // should not throw exception because limit was not overflown + _server.start(); + + Assert.fail(); + } + catch(IllegalStateException e) + { + Log.getLogger(ThreadBudget.class).warn(e.toString()); + } } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java index 6a582351cb1..1776d743188 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java @@ -18,7 +18,10 @@ package org.eclipse.jetty.util.component; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; +import java.util.Set; /** @@ -45,6 +48,7 @@ public interface Container * @return the list of beans of the given class (or subclass) * @param the Bean type * @see #getBeans() + * @see #getContainedBeans(Class) */ public Collection getBeans(Class clazz); @@ -93,6 +97,14 @@ public interface Container */ void manage(Object bean); + + /** + * Test if this container manages a bean + * @param bean the bean to test + * @return whether this aggregate contains and manages the bean + */ + boolean isManaged(Object bean); + /** * Adds the given bean, explicitly managing it or not. * @@ -121,4 +133,11 @@ public interface Container public interface InheritedListener extends Listener { } + + /** + * @param clazz the class of the beans + * @return the list of beans of the given class from the entire managed hierarchy + * @param the Bean type + */ + public Collection getContainedBeans(Class clazz); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java index 533c1949537..5c9d177daf1 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java @@ -22,7 +22,9 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.util.annotation.ManagedObject; @@ -198,6 +200,7 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, * @param bean the bean to test * @return whether this aggregate contains and manages the bean */ + @Override public boolean isManaged(Object bean) { for (Bean b : _beans) @@ -761,6 +764,19 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, return _managed==Managed.MANAGED; } + public boolean isManageable() + { + switch(_managed) + { + case MANAGED: + return true; + case AUTO: + return _bean instanceof LifeCycle && ((LifeCycle)_bean).isStopped(); + default: + return false; + } + } + @Override public String toString() { @@ -822,4 +838,38 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, } } } + + + /** + * @param clazz the class of the beans + * @return the list of beans of the given class from the entire managed hierarchy + * @param the Bean type + */ + public Collection getContainedBeans(Class clazz) + { + Set beans = new HashSet<>(); + getContainedBeans(clazz, beans); + return beans; + } + + /** + * @param clazz the class of the beans + * @param the Bean type + * @param beans the collection to add beans of the given class from the entire managed hierarchy + */ + protected void getContainedBeans(Class clazz, Collection beans) + { + beans.addAll(getBeans(clazz)); + for (Container c : getBeans(Container.class)) + { + Bean bean = getBean(c); + if (bean!=null && bean.isManageable()) + { + if (c instanceof ContainerLifeCycle) + ((ContainerLifeCycle)c).getContainedBeans(clazz, beans); + else + beans.addAll(c.getContainedBeans(clazz)); + } + } + } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index efadbcf694f..2606e8c291b 100755 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -16,7 +16,6 @@ // ======================================================================== // - package org.eclipse.jetty.util.thread; import java.io.IOException; @@ -66,6 +65,7 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo private boolean _daemon = false; private boolean _detailedDump = false; private int _lowThreadsThreshold = 1; + private ThreadBudget _budget; public QueuedThreadPool() { @@ -106,6 +106,23 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo } _jobs=queue; _threadGroup=threadGroup; + _budget=new ThreadBudget(this); + } + + @Override + public ThreadBudget getThreadBudget() + { + return _budget; + } + + public void setThreadBudget(ThreadBudget budget) + { + if (budget!=null && budget.getSizedThreadPool()!=this) + throw new IllegalArgumentException(); + synchronized (this) + { + _budget = budget; + } } @Override @@ -184,6 +201,9 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo } } + if (_budget!=null) + _budget.reset(); + synchronized (_joinLock) { _joinLock.notifyAll(); @@ -563,7 +583,7 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo @Override public String toString() { - return String.format("org.eclipse.jetty.util.thread.QueuedThreadPool@%s{%s,%d<=%d<=%d,i=%d,q=%d}", _name, getState(), getMinThreads(), getThreads(), getMaxThreads(), getIdleThreads(), (_jobs == null ? -1 : _jobs.size())); + return String.format("QueuedThreadPool@%s{%s,%d<=%d<=%d,i=%d,q=%d}", _name, getState(), getMinThreads(), getThreads(), getMaxThreads(), getIdleThreads(), (_jobs == null ? -1 : _jobs.size())); } private Runnable idleJobPoll() throws InterruptedException diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java index 6dd85adcd25..67f880e02a6 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java @@ -45,6 +45,8 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo private int _head; private int _size; private int _pending; + private ThreadBudget.Lease _lease; + private Object _owner; public ReservedThreadExecutor(Executor executor) { @@ -58,24 +60,42 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo * thread pool size. */ public ReservedThreadExecutor(Executor executor,int capacity) + { + this(executor,capacity,null); + } + + /** + * @param executor The executor to use to obtain threads + * @param capacity The number of threads to preallocate. If less than 0 then capacity + * is calculated based on a heuristic from the number of available processors and + * thread pool size. + */ + public ReservedThreadExecutor(Executor executor,int capacity, Object owner) { _executor = executor; + _queue = new ReservedThread[reservedThreads(executor,capacity)]; + _owner = owner; + } - if (capacity < 0) + /** + * @param executor The executor to use to obtain threads + * @param capacity The number of threads to preallocate, If less than 0 then capacity + * is calculated based on a heuristic from the number of available processors and + * thread pool size. + * @return the number of reserved threads that would be used by a ReservedThreadExecutor + * constructed with these arguments. + */ + public static int reservedThreads(Executor executor,int capacity) + { + if (capacity>=0) + return capacity; + int cpus = Runtime.getRuntime().availableProcessors(); + if (executor instanceof ThreadPool.SizedThreadPool) { - int cpus = Runtime.getRuntime().availableProcessors(); - if (executor instanceof ThreadPool.SizedThreadPool) - { - int threads = ((ThreadPool.SizedThreadPool)executor).getMaxThreads(); - capacity = Math.max(1, Math.min(cpus, threads / 8)); - } - else - { - capacity = cpus; - } + int threads = ((ThreadPool.SizedThreadPool)executor).getMaxThreads(); + return Math.max(1, Math.min(cpus, threads / 8)); } - - _queue = new ReservedThread[capacity]; + return cpus; } public Executor getExecutor() @@ -107,9 +127,18 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo } } + @Override + public void doStart() throws Exception + { + _lease = ThreadBudget.leaseFrom(getExecutor(),this,_queue.length); + super.doStart(); + } + @Override public void doStop() throws Exception { + if (_lease!=null) + _lease.close(); try (Locker.Lock lock = _locker.lock()) { while (_size>0) @@ -179,7 +208,9 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo { try (Locker.Lock lock = _locker.lock()) { - return String.format("%s{s=%d,p=%d}",super.toString(),_size,_pending); + if (_owner==null) + return String.format("%s@%x{s=%d,p=%d}",this.getClass().getSimpleName(),hashCode(),_size,_pending); + return String.format("%s@%s{s=%d,p=%d}",this.getClass().getSimpleName(),_owner,_size,_pending); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadBudget.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadBudget.java new file mode 100644 index 00000000000..1d9d0033007 --- /dev/null +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadBudget.java @@ -0,0 +1,178 @@ +// +// ======================================================================== +// Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.util.thread; + +import java.io.Closeable; +import java.io.IOException; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArraySet; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + +/** + *

A budget of required thread usage, used to warn or error for insufficient configured threads.

+ * + * @see ThreadPool.SizedThreadPool#getThreadBudget() + */ +public class ThreadBudget +{ + static final Logger LOG = Log.getLogger(ThreadBudget.class); + + public interface Lease extends Closeable + { + int getThreads(); + } + + /** + * An allocation of threads + */ + public class Leased implements Lease + { + final Object leasee; + final int threads; + + private Leased(Object leasee,int threads) + { + this.leasee = leasee; + this.threads = threads; + } + + @Override + public int getThreads() + { + return threads; + } + + @Override + public void close() + { + info.remove(this); + allocations.remove(this); + warned.set(false); + } + } + + private static final Lease NOOP_LEASE = new Lease() + { + @Override + public void close() throws IOException + { + } + + @Override + public int getThreads() + { + return 0; + } + }; + + final ThreadPool.SizedThreadPool pool; + final Set allocations = new CopyOnWriteArraySet<>(); + final Set info = new CopyOnWriteArraySet<>(); + final AtomicBoolean warned = new AtomicBoolean(); + final int warnAt; + + /** + * Construct a bedget for a SizedThreadPool, with the warning level set by heuristic. + * @param pool The pool to budget thread allocation for. + */ + public ThreadBudget(ThreadPool.SizedThreadPool pool) + { + this(pool,Runtime.getRuntime().availableProcessors()); + } + + /** + * @param pool The pool to budget thread allocation for. + * @param warnAt The level of free threads at which a warning is generated. + */ + public ThreadBudget(ThreadPool.SizedThreadPool pool, int warnAt) + { + this.pool = pool; + this.warnAt = warnAt; + } + + public ThreadPool.SizedThreadPool getSizedThreadPool() + { + return pool; + } + + public void reset() + { + allocations.clear(); + info.clear(); + warned.set(false); + } + + public Lease leaseTo(Object leasee, int threads) + { + Leased lease = new Leased(leasee,threads); + allocations.add(lease); + check(); + return lease; + } + + /** + * Check registered allocations against the budget. + * @throws IllegalStateException if insufficient threads are configured. + */ + public void check() throws IllegalStateException + { + int required = allocations.stream() + .mapToInt(Lease::getThreads) + .sum(); + + int maximum = pool.getMaxThreads(); + + if (required>=maximum) + { + infoOnLeases(); + throw new IllegalStateException(String.format("Insuffient configured threads: required=%d < max=%d for %s", required, maximum, pool)); + } + + if ((maximum-required) < warnAt) + { + infoOnLeases(); + if (warned.compareAndSet(false,true)) + LOG.warn("Low configured threads: ( max={} - required={} ) < warnAt={} for {}", maximum, required, warnAt, pool); + } + } + + private void infoOnLeases() + { + allocations.stream().filter(lease->!info.contains(lease)) + .forEach(lease->{ + info.add(lease); + LOG.info("{} requires {} threads from {}",lease.leasee,lease.getThreads(),pool); + }); + } + + public static Lease leaseFrom(Executor executor, Object leasee, int threads) + { + if (executor instanceof ThreadPool.SizedThreadPool) + { + ThreadBudget budget = ((ThreadPool.SizedThreadPool)executor).getThreadBudget(); + if (budget!=null) + return budget.leaseTo(leasee,threads); + } + return NOOP_LEASE; + } +} diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java index 56055793106..2bfafe13a80 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java @@ -67,9 +67,14 @@ public interface ThreadPool extends Executor /* ------------------------------------------------------------ */ public interface SizedThreadPool extends ThreadPool { - public int getMinThreads(); - public int getMaxThreads(); - public void setMinThreads(int threads); - public void setMaxThreads(int threads); + int getMinThreads(); + int getMaxThreads(); + void setMinThreads(int threads); + void setMaxThreads(int threads); + + default ThreadBudget getThreadBudget() + { + return null; + } } } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java index cdc610a7e2e..d16df62e7db 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java @@ -26,9 +26,13 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.jetty.util.TypeUtil; +import org.hamcrest.Matchers; import org.junit.Assert; import org.junit.Test; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.junit.Assert.assertThat; + public class ContainerLifeCycleTest { @Test @@ -608,4 +612,32 @@ public class ContainerLifeCycleTest super.destroy(); } } + + + @Test + public void testGetBeans() throws Exception + { + TestContainerLifeCycle root = new TestContainerLifeCycle(); + TestContainerLifeCycle left = new TestContainerLifeCycle(); + root.addBean(left); + TestContainerLifeCycle right = new TestContainerLifeCycle(); + root.addBean(right); + TestContainerLifeCycle leaf = new TestContainerLifeCycle(); + right.addBean(leaf); + + root.addBean(Integer.valueOf(0)); + root.addBean(Integer.valueOf(1)); + left.addBean(Integer.valueOf(2)); + right.addBean(Integer.valueOf(3)); + leaf.addBean(Integer.valueOf(4)); + leaf.addBean("leaf"); + + assertThat(root.getBeans(Container.class), containsInAnyOrder(left,right)); + assertThat(root.getBeans(Integer.class), containsInAnyOrder(Integer.valueOf(0),Integer.valueOf(1))); + assertThat(root.getBeans(String.class), containsInAnyOrder()); + + assertThat(root.getContainedBeans(Container.class), containsInAnyOrder(left,right,leaf)); + assertThat(root.getContainedBeans(Integer.class), containsInAnyOrder(Integer.valueOf(0),Integer.valueOf(1),Integer.valueOf(2),Integer.valueOf(3),Integer.valueOf(4))); + assertThat(root.getContainedBeans(String.class), containsInAnyOrder("leaf")); + } } diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java index 9859ec878fc..b7a020d59e2 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientTest.java @@ -37,6 +37,7 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.util.BytesContentProvider; From d166881a5a6bfbcb40e5d2734898eec3c74b8050 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 28 Sep 2017 12:44:23 +1000 Subject: [PATCH 04/16] Issue #1806 Concurrent ReservedThreadExcecutor Squashed commit of the following: commit ab7f711c30ad1408ecdceffdca51cf167dc11a9c Author: Greg Wilkins Date: Thu Sep 28 10:20:08 2017 +1000 Fixed format commit 062b051b56efb9924b81cf755297f538e4691851 Merge: d1e27d4 7d98cbb Author: Greg Wilkins Date: Thu Sep 28 09:19:40 2017 +1000 Merge branch 'jetty-9.4.x' into jetty-9.4.x-1806-ConcurrentReservedThreadExecutor commit d1e27d42983487941c6dd7cb0ea1068f36ff92f0 Author: Greg Wilkins Date: Tue Sep 26 11:28:21 2017 +1000 improvements after review commit 477e3ac05610825cc834d3195e39e8e8620a44cb Merge: 51a3ac3 35d0b59 Author: Greg Wilkins Date: Tue Sep 26 10:10:30 2017 +1000 Merge branch 'jetty-9.4.x' into jetty-9.4.x-1806-ConcurrentReservedThreadExecutor commit 51a3ac37e63108ca93fe2fdd9e02e7f8161072ba Author: Greg Wilkins Date: Tue Sep 26 10:08:55 2017 +1000 improvements after review commit 23df855bf457fe609576a35327db3fbd4c82b491 Merge: b52c6de 5764afc Author: Greg Wilkins Date: Tue Sep 26 09:30:42 2017 +1000 Merge branch 'jetty-9.4.x' into jetty-9.4.x-1806-ConcurrentReservedThreadExecutor commit b52c6de6573433a960fdb6f2692e7bbd9effb48c Author: Greg Wilkins Date: Fri Sep 22 15:36:53 2017 +1000 Issue #1806 Added a concurrent stack class Converted ReservedThreadExcecutor to use the concurrent stack Added support for idle --- .../jetty/server/NotAcceptingTest.java | 22 +- .../eclipse/jetty/util/ConcurrentStack.java | 91 ++++++ .../util/thread/ReservedThreadExecutor.java | 306 ++++++++++++------ .../thread/ReservedThreadExecutorTest.java | 167 +++++++++- .../test/resources/jetty-logging.properties | 3 +- 5 files changed, 485 insertions(+), 104 deletions(-) create mode 100644 jetty-util/src/main/java/org/eclipse/jetty/util/ConcurrentStack.java diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/NotAcceptingTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/NotAcceptingTest.java index 6a439fe2880..4574604f6d4 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/NotAcceptingTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/NotAcceptingTest.java @@ -36,17 +36,33 @@ import org.eclipse.jetty.server.LocalConnector.LocalEndPoint; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.toolchain.test.AdvancedRunner; import org.eclipse.jetty.util.BufferUtil; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @RunWith(AdvancedRunner.class) public class NotAcceptingTest { + Server server; + + @Before + public void before() + { + server = new Server(); + } + + @After + public void after() throws Exception + { + server.stop(); + server=null; + } + @Test public void testServerConnectorBlockingAccept() throws Exception { - Server server = new Server(); ServerConnector connector = new ServerConnector(server,1,1); connector.setPort(0); connector.setIdleTimeout(500); @@ -121,7 +137,6 @@ public class NotAcceptingTest @Test public void testLocalConnector() throws Exception { - Server server = new Server(); LocalConnector connector = new LocalConnector(server); connector.setIdleTimeout(500); server.addConnector(connector); @@ -174,7 +189,7 @@ public class NotAcceptingTest { // Can we accept the original? connector.setAccepting(true); - uri = handler.exchange.exchange("delayed connection"); + uri = handler.exchange.exchange("delayed connection",10,TimeUnit.SECONDS); assertThat(uri,is("/four")); response = HttpTester.parseResponse(client2.getResponse()); assertThat(response.getStatus(),is(200)); @@ -188,7 +203,6 @@ public class NotAcceptingTest @Test public void testServerConnectorAsyncAccept() throws Exception { - Server server = new Server(); ServerConnector connector = new ServerConnector(server,0,1); connector.setPort(0); connector.setIdleTimeout(500); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ConcurrentStack.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ConcurrentStack.java new file mode 100644 index 00000000000..a25be9306ac --- /dev/null +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ConcurrentStack.java @@ -0,0 +1,91 @@ +// +// ======================================================================== +// Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.util; + +import java.util.concurrent.atomic.AtomicReference; + +/** + * ConcurrentStack + * + * Nonblocking stack using variation of Treiber's algorithm + * that allows for reduced garbage + */ +public class ConcurrentStack +{ + private final NodeStack stack = new NodeStack<>(); + + public void push(I item) + { + stack.push(new Holder(item)); + } + + public I pop() + { + Holder holder = stack.pop(); + if (holder==null) + return null; + return holder.item; + } + + private static class Holder extends Node + { + final I item; + + Holder(I item) + { + this.item = item; + } + } + + public static class Node + { + Node next; + } + + public static class NodeStack + { + AtomicReference stack = new AtomicReference(); + + public void push(N node) + { + while(true) + { + Node top = stack.get(); + node.next = top; + if (stack.compareAndSet(top,node)) + break; + } + } + + public N pop() + { + while (true) + { + Node top = stack.get(); + if (top==null) + return null; + if (stack.compareAndSet(top,top.next)) + { + top.next = null; + return (N)top; + } + } + } + } +} diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java index 67f880e02a6..d58c69b2806 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java @@ -20,8 +20,11 @@ package org.eclipse.jetty.util.thread; import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.Condition; +import org.eclipse.jetty.util.ConcurrentStack; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.AbstractLifeCycle; @@ -32,21 +35,39 @@ import org.eclipse.jetty.util.log.Logger; * An Executor using preallocated/reserved Threads from a wrapped Executor. *

Calls to {@link #execute(Runnable)} on a {@link ReservedThreadExecutor} will either succeed * with a Thread immediately being assigned the Runnable task, or fail if no Thread is - * available. Threads are preallocated up to the capacity from a wrapped {@link Executor}. + * available. + *

Threads are reserved lazily, with a new reserved thread being allocated from a + * wrapped {@link Executor} when an execution fails. If the {@link #setIdleTimeout(long, TimeUnit)} + * is set to non zero (default 1 minute), then the reserved thread pool will shrink by 1 thread + * whenever it has been idle for that period. */ @ManagedObject("A pool for reserved threads") public class ReservedThreadExecutor extends AbstractLifeCycle implements Executor { private static final Logger LOG = Log.getLogger(ReservedThreadExecutor.class); + private static final Runnable STOP = new Runnable() + { + @Override + public void run() + {} + + @Override + public String toString() + { + return "STOP!"; + } + }; private final Executor _executor; - private final Locker _locker = new Locker(); - private final ReservedThread[] _queue; - private int _head; - private int _size; - private int _pending; + private final int _capacity; + private final ConcurrentStack.NodeStack _stack; + private final AtomicInteger _size = new AtomicInteger(); + private final AtomicInteger _pending = new AtomicInteger(); + private ThreadBudget.Lease _lease; private Object _owner; + private long _idleTime = 1L; + private TimeUnit _idleTimeUnit = TimeUnit.MINUTES; public ReservedThreadExecutor(Executor executor) { @@ -59,7 +80,7 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo * is calculated based on a heuristic from the number of available processors and * thread pool size. */ - public ReservedThreadExecutor(Executor executor,int capacity) + public ReservedThreadExecutor(Executor executor, int capacity) { this(executor,capacity,null); } @@ -73,10 +94,12 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo public ReservedThreadExecutor(Executor executor,int capacity, Object owner) { _executor = executor; - _queue = new ReservedThread[reservedThreads(executor,capacity)]; + _capacity = reservedThreads(executor,capacity); + _stack = new ConcurrentStack.NodeStack<>(); _owner = owner; - } + LOG.debug("{}",this); + } /** * @param executor The executor to use to obtain threads * @param capacity The number of threads to preallocate, If less than 0 then capacity @@ -106,31 +129,46 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo @ManagedAttribute(value = "max number of reserved threads", readonly = true) public int getCapacity() { - return _queue.length; + return _capacity; } @ManagedAttribute(value = "available reserved threads", readonly = true) public int getAvailable() { - try (Locker.Lock lock = _locker.lock()) - { - return _size; - } + return _size.get(); } @ManagedAttribute(value = "pending reserved threads", readonly = true) public int getPending() { - try (Locker.Lock lock = _locker.lock()) - { - return _pending; - } + return _pending.get(); + } + + @ManagedAttribute(value = "idletimeout in MS", readonly = true) + public long getIdleTimeoutMs() + { + if(_idleTimeUnit==null) + return 0; + return _idleTimeUnit.toMillis(_idleTime); + } + + /** + * Set the idle timeout for shrinking the reserved thread pool + * @param idleTime Time to wait before shrinking, or 0 for no timeout. + * @param idleTimeUnit Time units for idle timeout + */ + public void setIdleTimeout(long idleTime, TimeUnit idleTimeUnit) + { + if (isRunning()) + throw new IllegalStateException(); + _idleTime = idleTime; + _idleTimeUnit = idleTimeUnit; } @Override public void doStart() throws Exception { - _lease = ThreadBudget.leaseFrom(getExecutor(),this,_queue.length); + _lease = ThreadBudget.leaseFrom(getExecutor(),this,_capacity); super.doStart(); } @@ -139,16 +177,18 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo { if (_lease!=null) _lease.close(); - try (Locker.Lock lock = _locker.lock()) + while(true) { - while (_size>0) + ReservedThread thread = _stack.pop(); + if (thread==null) { - ReservedThread thread = _queue[_head]; - _queue[_head] = null; - _head = (_head+1)%_queue.length; - _size--; - thread._wakeup.signal(); + super.doStop(); + return; } + + _size.decrementAndGet(); + + thread.stop(); } } @@ -165,120 +205,196 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo */ public boolean tryExecute(Runnable task) { + if (LOG.isDebugEnabled()) + LOG.debug("{} tryExecute {}",this ,task); + if (task==null) return false; - try (Locker.Lock lock = _locker.lock()) + ReservedThread thread = _stack.pop(); + if (thread==null && task!=STOP) { - if (_size==0) + startReservedThread(); + return false; + } + + int size = _size.decrementAndGet(); + thread.offer(task); + + if (size==0 && task!=STOP) + startReservedThread(); + + return true; + } + + private void startReservedThread() + { + try + { + while (true) { - if (_pending<_queue.length) + int pending = _pending.get(); + if (pending >= _capacity) + return; + if (_pending.compareAndSet(pending, pending + 1)) { + if (LOG.isDebugEnabled()) + LOG.debug("{} startReservedThread p={}", this, pending + 1); + _executor.execute(new ReservedThread()); - _pending++; + return; } - return false; } - - ReservedThread thread = _queue[_head]; - _queue[_head] = null; - _head = (_head+1)%_queue.length; - _size--; - - if (_size==0 && _pending<_queue.length) - { - _executor.execute(new ReservedThread()); - _pending++; - } - - thread._task = task; - thread._wakeup.signal(); - - return true; } catch(RejectedExecutionException e) { LOG.ignore(e); - return false; } } @Override public String toString() { - try (Locker.Lock lock = _locker.lock()) - { - if (_owner==null) - return String.format("%s@%x{s=%d,p=%d}",this.getClass().getSimpleName(),hashCode(),_size,_pending); - return String.format("%s@%s{s=%d,p=%d}",this.getClass().getSimpleName(),_owner,_size,_pending); - } + if (_owner==null) + return String.format("%s@%x{s=%d,p=%d}",this.getClass().getSimpleName(),hashCode(),_size.get(),_pending.get()); + return String.format("%s@%s{s=%d,p=%d}",this.getClass().getSimpleName(),_owner,_size.get(),_pending.get()); } - private class ReservedThread implements Runnable + private class ReservedThread extends ConcurrentStack.Node implements Runnable { - private Condition _wakeup = null; + private final Locker _locker = new Locker(); + private final Condition _wakeup = _locker.newCondition(); + private boolean _starting = true; private Runnable _task = null; - private void reservedWait() throws InterruptedException + public void offer(Runnable task) { - _wakeup.await(); + if (LOG.isDebugEnabled()) + LOG.debug("{} offer {}", this, task); + + try (Locker.Lock lock = _locker.lock()) + { + _task = task; + _wakeup.signal(); + } } - @Override - public void run() + public void stop() { - while (true) + offer(STOP); + } + + private Runnable reservedWait() + { + if (LOG.isDebugEnabled()) + LOG.debug("{} waiting", this); + + Runnable task = null; + while (isRunning() && task==null) { - Runnable task = null; + boolean idle = false; + try (Locker.Lock lock = _locker.lock()) { - // if this is our first loop, decrement pending count - if (_wakeup==null) - { - _pending--; - _wakeup = _locker.newCondition(); - } - - // Exit if no longer running or there now too many preallocated threads - if (!isRunning() || _size>=_queue.length) - break; - - // Insert ourselves in the queue - _queue[(_head+_size++)%_queue.length] = this; - - // Wait for a task, ignoring spurious wakeups - while (isRunning() && task==null) + if (_task == null) { try { - if (LOG.isDebugEnabled()) - LOG.debug("{} waiting", this); - reservedWait(); - if (LOG.isDebugEnabled()) - LOG.debug("{} woken up", this); - task = _task; - _task = null; + if (_idleTime == 0) + _wakeup.await(); + else + idle = !_wakeup.await(_idleTime, _idleTimeUnit); } catch (InterruptedException e) { LOG.ignore(e); } } + task = _task; + _task = null; } - // Run any task - if (task!=null) + if (idle) { - try - { - task.run(); - } - catch (Throwable e) - { - LOG.warn(e); - } + // Because threads are held in a stack, excess threads will be + // idle. However, we cannot remove threads from the bottom of + // the stack, so we submit a poison pill job to stop the thread + // on top of the stack (which unfortunately will be the most + // recently used) + if (LOG.isDebugEnabled()) + LOG.debug("{} IDLE", this); + tryExecute(STOP); } } + + if (LOG.isDebugEnabled()) + LOG.debug("{} task={}", this, task); + + return task==null?STOP:task; + } + + @Override + public void run() + { + while (isRunning()) + { + Runnable task = null; + + // test and increment size BEFORE decrementing pending, + // so that we don't have a race starting new pending. + while(true) + { + int size = _size.get(); + if (size>=_capacity) + { + if (LOG.isDebugEnabled()) + LOG.debug("{} size {} > capacity", this, size, _capacity); + if (_starting) + _pending.decrementAndGet(); + return; + } + if (_size.compareAndSet(size,size+1)) + break; + } + + if (_starting) + { + if (LOG.isDebugEnabled()) + LOG.debug("{} started", this); + _pending.decrementAndGet(); + _starting = false; + } + + // Insert ourselves in the stack. Size is already incremented, but + // that only effects the decision to keep other threads reserved. + _stack.push(this); + + // Wait for a task + task = reservedWait(); + + if (task==STOP) + // return on STOP poison pill + break; + + // Run the task + try + { + task.run(); + } + catch (Throwable e) + { + LOG.warn(e); + } + } + + if (LOG.isDebugEnabled()) + LOG.debug("{} Exited", this); + } + + @Override + public String toString() + { + return String.format("%s@%x",ReservedThreadExecutor.this,hashCode()); } } } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/ReservedThreadExecutorTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/ReservedThreadExecutorTest.java index 3a3f73325dd..4b1964cdcfd 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/ReservedThreadExecutorTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/ReservedThreadExecutorTest.java @@ -18,24 +18,44 @@ package org.eclipse.jetty.util.thread; +import java.security.SecureRandom; import java.util.ArrayDeque; import java.util.Deque; +import java.util.Random; +import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.CyclicBarrier; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.StreamSupport; +import org.eclipse.jetty.toolchain.test.annotation.Stress; import org.junit.After; import org.junit.Assert; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; public class ReservedThreadExecutorTest { private static final int SIZE = 2; - private static final Runnable NOOP = () -> {}; + private static final Runnable NOOP = new Runnable() + { + @Override + public void run() + {} + + @Override + public String toString() + { + return "NOOP!"; + } + }; private TestExecutor _executor; private ReservedThreadExecutor _reservedExecutor; @@ -127,17 +147,87 @@ public class ReservedThreadExecutorTest waitForAllAvailable(); } - protected void waitForAllAvailable() throws InterruptedException + + @Test + public void testShrink() throws Exception + { + final long IDLE = 1000; + + _reservedExecutor.stop(); + _reservedExecutor.setIdleTimeout(IDLE,TimeUnit.MILLISECONDS); + _reservedExecutor.start(); + + // Reserved threads are lazily started. + assertThat(_executor._queue.size(), is(0)); + + assertThat(_reservedExecutor.tryExecute(NOOP),is(false)); + _executor.execute(); + waitForNoPending(); + + CountDownLatch latch = new CountDownLatch(1); + Runnable waitForLatch = ()->{try {latch.await();} catch(Exception e){}}; + assertThat(_reservedExecutor.tryExecute(waitForLatch),is(true)); + _executor.execute(); + + assertThat(_reservedExecutor.tryExecute(NOOP),is(false)); + _executor.execute(); + waitForNoPending(); + + latch.countDown(); + waitForAvailable(2); + + // Check that regular moderate activity keeps the pool a moderate size + TimeUnit.MILLISECONDS.sleep(IDLE/2); + assertThat(_reservedExecutor.tryExecute(NOOP),is(true)); + waitForAvailable(2); + TimeUnit.MILLISECONDS.sleep(IDLE/2); + assertThat(_reservedExecutor.tryExecute(NOOP),is(true)); + waitForAvailable(1); + TimeUnit.MILLISECONDS.sleep(IDLE/2); + assertThat(_reservedExecutor.tryExecute(NOOP),is(true)); + waitForAvailable(1); + TimeUnit.MILLISECONDS.sleep(IDLE/2); + assertThat(_reservedExecutor.tryExecute(NOOP),is(true)); + waitForAvailable(1); + TimeUnit.MILLISECONDS.sleep(IDLE/2); + assertThat(_reservedExecutor.tryExecute(NOOP),is(true)); + waitForAvailable(1); + + // check fully idle goes to zero + TimeUnit.MILLISECONDS.sleep(IDLE); + assertThat(_reservedExecutor.getAvailable(),is(0)); + + } + + protected void waitForNoPending() throws InterruptedException { long started = System.nanoTime(); - while (_reservedExecutor.getAvailable() < SIZE) + while (_reservedExecutor.getPending() > 0) + { + long elapsed = System.nanoTime() - started; + if (elapsed > TimeUnit.SECONDS.toNanos(10)) + Assert.fail("pending="+_reservedExecutor.getPending()); + Thread.sleep(10); + } + assertThat(_reservedExecutor.getPending(), is(0)); + } + + protected void waitForAvailable(int size) throws InterruptedException + { + long started = System.nanoTime(); + while (_reservedExecutor.getAvailable() < size) { long elapsed = System.nanoTime() - started; if (elapsed > TimeUnit.SECONDS.toNanos(10)) Assert.fail(); Thread.sleep(10); } - assertThat(_reservedExecutor.getAvailable(), is(SIZE)); + assertThat(_reservedExecutor.getAvailable(), is(size)); + } + + protected void waitForAllAvailable() throws InterruptedException + { + waitForAvailable(SIZE); } private static class TestExecutor implements Executor @@ -177,4 +267,73 @@ public class ReservedThreadExecutorTest } } } + + @Ignore + @Test + public void stressTest() throws Exception + { + QueuedThreadPool pool = new QueuedThreadPool(20); + pool.setStopTimeout(10000); + pool.start(); + ReservedThreadExecutor reserved = new ReservedThreadExecutor(pool,10); + reserved.setIdleTimeout(0,null); + reserved.start(); + + final int LOOPS = 1000000; + final Random random = new Random(); + final AtomicInteger executions = new AtomicInteger(LOOPS); + final CountDownLatch executed = new CountDownLatch(executions.get()); + final AtomicInteger usedReserved = new AtomicInteger(0); + final AtomicInteger usedPool = new AtomicInteger(0); + + Runnable task = new Runnable() + { + public void run() + { + try + { + while (true) + { + int loops = executions.get(); + if (loops <= 0) + return; + + if (executions.compareAndSet(loops, loops - 1)) + { + if (reserved.tryExecute(this)) + { + usedReserved.incrementAndGet(); + } else + { + usedPool.incrementAndGet(); + pool.execute(this); + } + return; + } + } + } + finally + { + executed.countDown(); + } + } + }; + + task.run(); + task.run(); + task.run(); + task.run(); + task.run(); + task.run(); + task.run(); + task.run(); + + assertTrue(executed.await(60,TimeUnit.SECONDS)); + + reserved.stop(); + pool.stop(); + + assertThat(usedReserved.get()+usedPool.get(),is(LOOPS)); + System.err.printf("reserved=%d pool=%d total=%d%n",usedReserved.get(),usedPool.get(),LOOPS); + } } diff --git a/jetty-util/src/test/resources/jetty-logging.properties b/jetty-util/src/test/resources/jetty-logging.properties index adc470de2b3..96eb0ce650a 100644 --- a/jetty-util/src/test/resources/jetty-logging.properties +++ b/jetty-util/src/test/resources/jetty-logging.properties @@ -1,4 +1,5 @@ # Setup default logging implementation for during testing org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog #org.eclipse.jetty.util.LEVEL=DEBUG -#org.eclipse.jetty.util.PathWatcher.LEVEL=DEBUG \ No newline at end of file +#org.eclipse.jetty.util.PathWatcher.LEVEL=DEBUG +#org.eclipse.jetty.util.thread.ReservedThreadExecutor.LEVEL=DEBUG From d5b55705bf31162b70740820bc837eaddd13876d Mon Sep 17 00:00:00 2001 From: WalkerWatch Date: Fri, 29 Sep 2017 14:43:29 -0400 Subject: [PATCH 05/16] Added javadoc.version value to assis with snapshot javadoc links in #1858 --- jetty-documentation/pom.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jetty-documentation/pom.xml b/jetty-documentation/pom.xml index 7d5afd190d3..fc1c7cd948c 100644 --- a/jetty-documentation/pom.xml +++ b/jetty-documentation/pom.xml @@ -11,6 +11,7 @@ pom ${project.build.directory}/current + ${project.version} @@ -58,7 +59,7 @@ true true ${project.version} - http://www.eclipse.org/jetty/javadoc/${project.version} + http://www.eclipse.org/jetty/javadoc/${javadoc.version} http://download.eclipse.org/jetty/stable-9/xref ${basedir}/.. https://github.com/eclipse/jetty.project/tree/jetty-9.4.x From b3ddf2b15ea02f524006d09eea2d6d39ffbbd88e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 30 Sep 2017 22:14:36 +0200 Subject: [PATCH 06/16] Issue #487 - JDK 9 build compatibility. Updated Maven Surefire and Failsafe Plugin to 2.20.1 following https://issues.apache.org/jira/browse/SUREFIRE-1403. --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 3691716c44b..e0ed70857cd 100644 --- a/pom.xml +++ b/pom.xml @@ -416,7 +416,7 @@ org.apache.maven.plugins maven-failsafe-plugin - 2.20 + 2.20.1 org.apache.maven.plugins @@ -570,7 +570,7 @@ org.apache.maven.plugins maven-surefire-plugin - 2.20 + 2.20.1 @{argLine} -Dfile.encoding=UTF-8 -Duser.language=en -Duser.region=US -showversion -Xmx1g -Xms1g -XX:+PrintGCDetails false From 3eaba140ef6e1cf7adc42181115a250bf2ecd4f3 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 30 Sep 2017 22:15:17 +0200 Subject: [PATCH 07/16] Fixed logging. --- .../jetty/alpn/java/client/OpenJDK8ClientALPNProcessor.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/jetty-alpn/jetty-alpn-openjdk8-client/src/main/java/org/eclipse/jetty/alpn/java/client/OpenJDK8ClientALPNProcessor.java b/jetty-alpn/jetty-alpn-openjdk8-client/src/main/java/org/eclipse/jetty/alpn/java/client/OpenJDK8ClientALPNProcessor.java index 539cb6cd872..9c320889974 100644 --- a/jetty-alpn/jetty-alpn-openjdk8-client/src/main/java/org/eclipse/jetty/alpn/java/client/OpenJDK8ClientALPNProcessor.java +++ b/jetty-alpn/jetty-alpn-openjdk8-client/src/main/java/org/eclipse/jetty/alpn/java/client/OpenJDK8ClientALPNProcessor.java @@ -40,7 +40,9 @@ public class OpenJDK8ClientALPNProcessor implements ALPNProcessor.Client if (JavaVersion.VERSION.getPlatform()!=8) throw new IllegalStateException(this + " not applicable for java "+JavaVersion.VERSION); if (ALPN.class.getClassLoader()!=null) - throw new IllegalStateException(this + " must be on JVM boot classpath"); + throw new IllegalStateException(ALPN.class.getName() + " must be on JVM boot classpath"); + if (LOG.isDebugEnabled()) + ALPN.debug = true; } @Override From a6e28b5ce0e4fa6a22f798dd215cb89b7c4b6533 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 30 Sep 2017 22:16:45 +0200 Subject: [PATCH 08/16] Issue #1859 - Jetty http2 client idle_timeout when trying to get the session after connected to Jetty HTTP2 server. Implemented an integration test case that shows HTTP2Client usage from within a web application. --- tests/test-webapps/pom.xml | 1 + tests/test-webapps/test-http2-webapp/pom.xml | 175 ++++++++++++++++++ .../jetty/test/webapp/HTTP1Servlet.java | 138 ++++++++++++++ .../jetty/test/webapp/HTTP2Servlet.java | 35 ++++ .../src/main/webapp/WEB-INF/web.xml | 28 +++ .../jetty/test/webapp/HTTP2FromWebAppIT.java | 96 ++++++++++ .../test/resources/jetty-logging.properties | 3 + .../src/test/resources/keystore.jks | Bin 0 -> 2206 bytes 8 files changed, 476 insertions(+) create mode 100644 tests/test-webapps/test-http2-webapp/pom.xml create mode 100644 tests/test-webapps/test-http2-webapp/src/main/java/org/eclipse/jetty/test/webapp/HTTP1Servlet.java create mode 100644 tests/test-webapps/test-http2-webapp/src/main/java/org/eclipse/jetty/test/webapp/HTTP2Servlet.java create mode 100644 tests/test-webapps/test-http2-webapp/src/main/webapp/WEB-INF/web.xml create mode 100644 tests/test-webapps/test-http2-webapp/src/test/java/org/eclipse/jetty/test/webapp/HTTP2FromWebAppIT.java create mode 100644 tests/test-webapps/test-http2-webapp/src/test/resources/jetty-logging.properties create mode 100644 tests/test-webapps/test-http2-webapp/src/test/resources/keystore.jks diff --git a/tests/test-webapps/pom.xml b/tests/test-webapps/pom.xml index 738e7695aa1..72a533eeae6 100644 --- a/tests/test-webapps/pom.xml +++ b/tests/test-webapps/pom.xml @@ -38,5 +38,6 @@ test-servlet-spec test-jaas-webapp test-jndi-webapp + test-http2-webapp diff --git a/tests/test-webapps/test-http2-webapp/pom.xml b/tests/test-webapps/test-http2-webapp/pom.xml new file mode 100644 index 00000000000..3b3ba85365b --- /dev/null +++ b/tests/test-webapps/test-http2-webapp/pom.xml @@ -0,0 +1,175 @@ + + + + org.eclipse.jetty.tests + test-webapps-parent + 9.4.8-SNAPSHOT + + + 4.0.0 + test-http2-webapp + Test :: Jetty HTTP2 Webapp + war + + + ${project.groupId}.http2 + + + + + + maven-dependency-plugin + + + unpack-webapp + pre-integration-test + + unpack + + + + + ${project.groupId} + ${project.artifactId} + ${project.version} + war + false + ${project.build.directory}/webapp + + + + + + + + maven-failsafe-plugin + + + + integration-test + verify + + + + + + + + + + jdk8 + + [1.8,1.9) + + + + + maven-dependency-plugin + + + copy-alpn-boot + pre-integration-test + + copy + + + + + org.mortbay.jetty.alpn + alpn-boot + ${alpn.version} + jar + false + ${project.build.directory}/alpn + + + + + + + + maven-failsafe-plugin + + -Xbootclasspath/p:${project.build.directory}/alpn/alpn-boot-${alpn.version}.jar + + + + + + + org.eclipse.jetty + jetty-alpn-openjdk8-client + ${project.version} + + + org.eclipse.jetty.alpn + alpn-api + + + + + org.eclipse.jetty + jetty-alpn-openjdk8-server + ${project.version} + test + + + + + jdk9 + + [1.9,) + + + + org.eclipse.jetty + jetty-alpn-java-client + ${project.version} + + + org.eclipse.jetty + jetty-alpn-java-server + ${project.version} + test + + + + + + + + org.eclipse.jetty.http2 + http2-client + ${project.version} + + + javax.servlet + javax.servlet-api + provided + + + + org.eclipse.jetty + jetty-client + ${project.version} + test + + + org.eclipse.jetty + jetty-webapp + ${project.version} + test + + + org.eclipse.jetty.http2 + http2-server + ${project.version} + test + + + junit + junit + test + + + diff --git a/tests/test-webapps/test-http2-webapp/src/main/java/org/eclipse/jetty/test/webapp/HTTP1Servlet.java b/tests/test-webapps/test-http2-webapp/src/main/java/org/eclipse/jetty/test/webapp/HTTP1Servlet.java new file mode 100644 index 00000000000..dd7aae745bc --- /dev/null +++ b/tests/test-webapps/test-http2-webapp/src/main/java/org/eclipse/jetty/test/webapp/HTTP1Servlet.java @@ -0,0 +1,138 @@ +// +// ======================================================================== +// Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.test.webapp; + +import java.io.IOException; +import java.net.InetSocketAddress; +import java.nio.ByteBuffer; + +import javax.servlet.AsyncContext; +import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpURI; +import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.http.MetaData; +import org.eclipse.jetty.http2.api.Session; +import org.eclipse.jetty.http2.api.Stream; +import org.eclipse.jetty.http2.client.HTTP2Client; +import org.eclipse.jetty.http2.frames.DataFrame; +import org.eclipse.jetty.http2.frames.HeadersFrame; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.ssl.SslContextFactory; + +public class HTTP1Servlet extends HttpServlet +{ + private SslContextFactory sslContextFactory; + private HTTP2Client http2Client; + + @Override + public void init() throws ServletException + { + try + { + sslContextFactory = new SslContextFactory(true); + http2Client = new HTTP2Client(); + http2Client.addBean(sslContextFactory); + http2Client.start(); + } + catch (Exception x) + { + throw new ServletException(x); + } + } + + @Override + public void destroy() + { + try + { + http2Client.stop(); + } + catch (Exception x) + { + x.printStackTrace(); + } + } + + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + String host = "localhost"; + int port = request.getServerPort(); + String contextPath = request.getContextPath(); + ServletOutputStream output = response.getOutputStream(); + AsyncContext asyncContext = request.startAsync(); + http2Client.connect(sslContextFactory, new InetSocketAddress(host, port), new Session.Listener.Adapter(), new Promise() + { + @Override + public void succeeded(Session session) + { + HttpURI uri = new HttpURI(request.getScheme(), host, port, contextPath + "/h2"); + MetaData.Request metaData = new MetaData.Request(HttpMethod.GET.asString(), uri, HttpVersion.HTTP_2, new HttpFields()); + HeadersFrame frame = new HeadersFrame(metaData, null, true); + session.newStream(frame, new Promise.Adapter() + { + @Override + public void failed(Throwable x) + { + response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500); + response.setHeader("X-Failure", "stream"); + asyncContext.complete(); + } + }, new Stream.Listener.Adapter() + { + @Override + public void onData(Stream stream, DataFrame frame, Callback callback) + { + try + { + ByteBuffer buffer = frame.getData(); + byte[] bytes = new byte[buffer.remaining()]; + buffer.get(bytes); + output.write(bytes); + callback.succeeded(); + if (frame.isEndStream()) + asyncContext.complete(); + } + catch (IOException x) + { + asyncContext.complete(); + } + } + }); + } + + @Override + public void failed(Throwable x) + { + response.setStatus(HttpStatus.INTERNAL_SERVER_ERROR_500); + response.setHeader("X-Failure", "session"); + asyncContext.complete(); + } + }); + } +} diff --git a/tests/test-webapps/test-http2-webapp/src/main/java/org/eclipse/jetty/test/webapp/HTTP2Servlet.java b/tests/test-webapps/test-http2-webapp/src/main/java/org/eclipse/jetty/test/webapp/HTTP2Servlet.java new file mode 100644 index 00000000000..ab92da37e01 --- /dev/null +++ b/tests/test-webapps/test-http2-webapp/src/main/java/org/eclipse/jetty/test/webapp/HTTP2Servlet.java @@ -0,0 +1,35 @@ +// +// ======================================================================== +// Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.test.webapp; + +import java.io.IOException; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +public class HTTP2Servlet extends HttpServlet +{ + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + { + response.getOutputStream().print("ok"); + } +} diff --git a/tests/test-webapps/test-http2-webapp/src/main/webapp/WEB-INF/web.xml b/tests/test-webapps/test-http2-webapp/src/main/webapp/WEB-INF/web.xml new file mode 100644 index 00000000000..9944a3b0ff2 --- /dev/null +++ b/tests/test-webapps/test-http2-webapp/src/main/webapp/WEB-INF/web.xml @@ -0,0 +1,28 @@ + + + + + h1 + org.eclipse.jetty.test.webapp.HTTP1Servlet + true + + + h1 + /h1 + + + + h2 + org.eclipse.jetty.test.webapp.HTTP2Servlet + + + h2 + /h2 + + + + + diff --git a/tests/test-webapps/test-http2-webapp/src/test/java/org/eclipse/jetty/test/webapp/HTTP2FromWebAppIT.java b/tests/test-webapps/test-http2-webapp/src/test/java/org/eclipse/jetty/test/webapp/HTTP2FromWebAppIT.java new file mode 100644 index 00000000000..b4f3a8a9391 --- /dev/null +++ b/tests/test-webapps/test-http2-webapp/src/test/java/org/eclipse/jetty/test/webapp/HTTP2FromWebAppIT.java @@ -0,0 +1,96 @@ +// +// ======================================================================== +// Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.test.webapp; + +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.http.HttpScheme; +import org.eclipse.jetty.http2.HTTP2Cipher; +import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory; +import org.eclipse.jetty.server.HttpConfiguration; +import org.eclipse.jetty.server.HttpConnectionFactory; +import org.eclipse.jetty.server.SecureRequestCustomizer; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.SslConnectionFactory; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.webapp.WebAppContext; +import org.junit.Assert; +import org.junit.Test; + +public class HTTP2FromWebAppIT +{ + @Test + public void testHTTP2FromWebApp() throws Exception + { + Server server = new Server(); + + SslContextFactory serverTLS = new SslContextFactory(); + serverTLS.setKeyStorePath("src/test/resources/keystore.jks"); + serverTLS.setKeyStorePassword("storepwd"); + serverTLS.setCipherComparator(new HTTP2Cipher.CipherComparator()); + + HttpConfiguration httpsConfig = new HttpConfiguration(); + httpsConfig.addCustomizer(new SecureRequestCustomizer()); + + HttpConnectionFactory h1 = new HttpConnectionFactory(httpsConfig); + ALPNServerConnectionFactory alpn = new ALPNServerConnectionFactory(); + alpn.setDefaultProtocol(h1.getProtocol()); + SslConnectionFactory ssl = new SslConnectionFactory(serverTLS, alpn.getProtocol()); + HTTP2ServerConnectionFactory h2 = new HTTP2ServerConnectionFactory(httpsConfig); + + ServerConnector connector = new ServerConnector(server, ssl, alpn, h2, h1); + server.addConnector(connector); + + String contextPath = "/http2_from_webapp"; + WebAppContext context = new WebAppContext("target/webapp", contextPath); + server.setHandler(context); + + server.start(); + + try + { + SslContextFactory clientTLS = new SslContextFactory(true); + HttpClient client = new HttpClient(clientTLS); + client.start(); + + try + { + ContentResponse response = client.newRequest("localhost", connector.getLocalPort()) + .scheme(HttpScheme.HTTPS.asString()) + .path(contextPath + "/h1") + .timeout(5, TimeUnit.SECONDS) + .send(); + + Assert.assertEquals("ok", response.getContentAsString()); + } + finally + { + client.stop(); + } + } + finally + { + server.stop(); + } + } +} diff --git a/tests/test-webapps/test-http2-webapp/src/test/resources/jetty-logging.properties b/tests/test-webapps/test-http2-webapp/src/test/resources/jetty-logging.properties new file mode 100644 index 00000000000..055e90b60ef --- /dev/null +++ b/tests/test-webapps/test-http2-webapp/src/test/resources/jetty-logging.properties @@ -0,0 +1,3 @@ +org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog +org.eclipse.jetty.LEVEL=INFO +#org.eclipse.jetty.alpn.LEVEL=DEBUG diff --git a/tests/test-webapps/test-http2-webapp/src/test/resources/keystore.jks b/tests/test-webapps/test-http2-webapp/src/test/resources/keystore.jks new file mode 100644 index 0000000000000000000000000000000000000000..428ba54776ede2fdcdeedd879edb927c2abd9953 GIT binary patch literal 2206 zcmcgt`9Bkm8{cNkoMUp6gmShKn!AQX*(l6Nj(i=TnQPOKYtv{*Wg>ItE=Q!pRYH8a z$Sp#S#2lYw#aw;$y9u4T}83H*%lp zAKZay0sy=q1Qoo85aAQh;$ zD(c2EIN#D7WwYDLKUg!CotQPD@dp;5FR#bgaace(^x$6g5frD~(_b(MI^J&*A2DRp zf5Q2onfE(zvUb9|9C`66)YFRNM6~xrz4;iVbU=P|*YT2eWHFJJtr+M@zt2qPm)K~rRcqcs=LM12)PX0TT%QO zlf*xkqD3}7l)1J`5W(>=9nR0e6j-<79<11v3ZuXXcQpoCsqY~n`$FN+S}hcVm5Y>G zXnD{@DYs1@{S0z(lW+?86LWKtku$$-(khsh>0qRUXn=84`GRn?77M^_JY`durnN;KE zW#OJ`h<6xcB{I))ekGpc*Ylt}0cx4|OMBDPQvx4`r`}4Ze5_ipdObGMTi3bZHd5PC zcY0;?uBWu$PSvjJeb87nY7ghNv?%M@SoDl6IWt`bQCosfSh$#D6$ea~QhKM^ud2Ut z+9PYJuVpoELmN-A`F$BicO{BSYg@#tS%avVfb}DxL)|NanJ)#zB!2~?#Ot%H7--9N zU$bs0fS5G!m5M4&WK3#a|H|Tgw*?X-;H+Lu@kwA>qSR~7UC7b)7MJXTn6PG>n@8jP zW+}F^X$$c;U~4ryqRF; z>`j!tbLMK4ZGyY643|~?%Mu#fm!l%wAKjBDmd+VYmp3S#$scD$~bxbf|z#)hShN0*AhRaPDcmqrftGlHq4^54MM$Xfy(2> zH8QYVMzmn_oHbvJCB`IN~E&{1*h&0gEM{e zKvWvzp(!BqMX8`t#)~0nq}Wa zr6>FRPyp;AAB&)1$5@;r$23J{K&~>TWjZf7V$wFzmGM95CXhFG1cJNVAXks}C+&2- zbf9Qn*D8N}Afd2kpwDxns3%1uaFhAqDV8ksWiWY|quuLGZ0)SqrJ!Y8yX}@}IyC$C zQ3rCUsn}#>F#D8%D?q~ySy4j&he%Bs{{7V%rl!ui`@KQP?NTi+_iN{cwom&9RaMRR zB~z!hz|0HAgB9_Ijvpe-zr#jLbckJsc>vmo{+im?t8lA;N#fD4?{lb&J0V8Gocq%; f1ihv=QIDh{M_<9V+45Z2{KE4_qW}V3B0uV%GgrOJ literal 0 HcmV?d00001 From 4526006bde988389904aea8a5560a2eacac1cab4 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 30 Sep 2017 22:46:23 +0200 Subject: [PATCH 09/16] Reduced logging. The ALPN callback may not be invoked in case the client does not support ALPN, so the server will use the default protocol. --- .../conscrypt/server/ConscryptServerALPNProcessor.java | 10 ++++------ .../alpn/java/server/JDK9ServerALPNProcessor.java | 10 ++++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/jetty-alpn/jetty-alpn-conscrypt-server/src/main/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptServerALPNProcessor.java b/jetty-alpn/jetty-alpn-conscrypt-server/src/main/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptServerALPNProcessor.java index 0d8acbbf874..06179ed0af6 100644 --- a/jetty-alpn/jetty-alpn-conscrypt-server/src/main/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptServerALPNProcessor.java +++ b/jetty-alpn/jetty-alpn-conscrypt-server/src/main/java/org/eclipse/jetty/alpn/conscrypt/server/ConscryptServerALPNProcessor.java @@ -96,20 +96,18 @@ public class ConscryptServerALPNProcessor implements ALPNProcessor.Server @Override public void handshakeSucceeded(Event event) { + String protocol = alpnConnection.getProtocol(); if (LOG.isDebugEnabled()) - LOG.debug("handshakeSucceeded {} {}", alpnConnection, event); - if (alpnConnection.getProtocol()==null) - { - LOG.warn("No ALPN callback! {} {}",alpnConnection, event); + LOG.debug("TLS handshake succeeded, protocol={} for {}", protocol, alpnConnection); + if (protocol ==null) alpnConnection.unsupported(); - } } @Override public void handshakeFailed(Event event, Throwable failure) { if (LOG.isDebugEnabled()) - LOG.debug("handshakeFailed {} {} {}", alpnConnection, event, failure); + LOG.debug("TLS handshake failed " + alpnConnection, failure); } } } diff --git a/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java b/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java index f9e0313a211..08cc301246e 100644 --- a/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java +++ b/jetty-alpn/jetty-alpn-java-server/src/main/java/org/eclipse/jetty/alpn/java/server/JDK9ServerALPNProcessor.java @@ -78,20 +78,18 @@ public class JDK9ServerALPNProcessor implements ALPNProcessor.Server, SslHandsha @Override public void handshakeSucceeded(Event event) { + String protocol = alpnConnection.getProtocol(); if (LOG.isDebugEnabled()) - LOG.debug("handshakeSucceeded {} {}", alpnConnection, event); - if (alpnConnection.getProtocol()==null) - { - LOG.warn("No ALPN callback! {} {}",alpnConnection, event); + LOG.debug("TLS handshake succeeded, protocol={} for {}", protocol, alpnConnection); + if (protocol ==null) alpnConnection.unsupported(); - } } @Override public void handshakeFailed(Event event, Throwable failure) { if (LOG.isDebugEnabled()) - LOG.debug("handshakeFailed {} {} {}", alpnConnection, event, failure); + LOG.debug("TLS handshake failed " + alpnConnection, failure); } } } From 17a1484143694927f59ed1c1eee8279e29f2e3fa Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 2 Oct 2017 11:43:45 +0200 Subject: [PATCH 10/16] Code cleanups. Removed unnecessary imports and fixed typos. --- .../java/org/eclipse/jetty/embedded/ManyConnectors.java | 1 - .../main/java/org/eclipse/jetty/client/HttpClient.java | 1 - .../main/java/org/eclipse/jetty/io/SelectorManager.java | 1 - .../src/main/java/org/eclipse/jetty/server/Server.java | 3 --- .../jetty/server/InsufficientThreadsDetectionTest.java | 9 +++------ .../java/org/eclipse/jetty/util/component/Container.java | 4 ---- 6 files changed, 3 insertions(+), 16 deletions(-) diff --git a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyConnectors.java b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyConnectors.java index 798434a74a7..1a81f776131 100644 --- a/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyConnectors.java +++ b/examples/embedded/src/main/java/org/eclipse/jetty/embedded/ManyConnectors.java @@ -32,7 +32,6 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.SslConnectionFactory; import org.eclipse.jetty.util.ssl.SslContextFactory; -import org.eclipse.jetty.util.thread.QueuedThreadPool; /** * A Jetty server with multiple connectors. 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 3deae692ce1..023da71bfd9 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 @@ -73,7 +73,6 @@ 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.eclipse.jetty.util.thread.ThreadBudget; import org.eclipse.jetty.util.thread.ThreadPool; /** diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index 467920df9bf..792610ea6b6 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -70,7 +70,6 @@ public abstract class SelectorManager extends ContainerLifeCycle implements Dump int cpus = Runtime.getRuntime().availableProcessors(); return Math.max(1,Math.min(cpus/2,threads/16)); } - return Math.max(1,Runtime.getRuntime().availableProcessors()/2); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index f476b58b9fd..d3807cfa175 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -24,7 +24,6 @@ import java.net.InetSocketAddress; import java.net.URI; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.Enumeration; @@ -66,9 +65,7 @@ import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.Locker; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.ShutdownThread; -import org.eclipse.jetty.util.thread.ThreadBudget; import org.eclipse.jetty.util.thread.ThreadPool; -import org.eclipse.jetty.util.thread.ThreadPool.SizedThreadPool; /* ------------------------------------------------------------ */ /** Jetty HTTP Servlet Server. diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java index c4e53c9bdf3..adce494570b 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java @@ -82,8 +82,6 @@ public class InsufficientThreadsDetectionTest _server.start(); } - // Github issue #586 - @Test public void testCaseForMultipleConnectors() throws Exception { @@ -95,14 +93,14 @@ public class InsufficientThreadsDetectionTest // first connector consumes 3 threads from server pool _server.addConnector(new ServerConnector(_server, null, null, null, 1, 1, new HttpConnectionFactory())); - // second connect also require 4 threads but uses own executor, so its threads should not be counted + // second connect also require 3 threads but uses own executor, so its threads should not be counted final QueuedThreadPool connectorPool = new QueuedThreadPool(4, 4); _server.addConnector(new ServerConnector(_server, connectorPool, null, null, 1, 1, new HttpConnectionFactory())); - // first connector consumes 3 threads from server pool + // third connector consumes 3 threads from server pool _server.addConnector(new ServerConnector(_server, null, null, null, 1, 1, new HttpConnectionFactory())); - // should not throw exception because limit was not overflown + // should throw exception because limit was overflown _server.start(); Assert.fail(); @@ -112,5 +110,4 @@ public class InsufficientThreadsDetectionTest Log.getLogger(ThreadBudget.class).warn(e.toString()); } } - } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java index 1776d743188..f7fde9964dd 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Container.java @@ -18,11 +18,7 @@ package org.eclipse.jetty.util.component; -import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; -import java.util.Set; - /** * A Container From a811785d64d080465357dffe1030e06a6cdc72fc Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 2 Oct 2017 13:16:44 +0200 Subject: [PATCH 11/16] Reverting Maven Surefire Plugin to 2.20. Version 2.20.1 suffers from https://issues.apache.org/jira/browse/SUREFIRE-1424, which breaks the Infinispan tests. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index e0ed70857cd..5694935b61f 100644 --- a/pom.xml +++ b/pom.xml @@ -570,7 +570,7 @@ org.apache.maven.plugins maven-surefire-plugin - 2.20.1 + 2.20 @{argLine} -Dfile.encoding=UTF-8 -Duser.language=en -Duser.region=US -showversion -Xmx1g -Xms1g -XX:+PrintGCDetails false From 3b98a6c000af555762f138470e99ea632b62910e Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 2 Oct 2017 13:40:31 +0200 Subject: [PATCH 12/16] Issue #1851 - Improve insufficient thread warnings/errors. ThreadBudget -> ThreadPoolBudget. Added selectors to the leased threads. --- .../InsufficientThreadsDetectionTest.java | 58 +++++++++++++++++++ .../org/eclipse/jetty/io/SelectorManager.java | 8 ++- .../jetty/server/AbstractConnector.java | 12 ++-- .../InsufficientThreadsDetectionTest.java | 6 +- .../jetty/util/thread/QueuedThreadPool.java | 13 ++--- .../util/thread/ReservedThreadExecutor.java | 4 +- .../eclipse/jetty/util/thread/ThreadPool.java | 3 +- ...hreadBudget.java => ThreadPoolBudget.java} | 24 ++++---- 8 files changed, 91 insertions(+), 37 deletions(-) create mode 100644 jetty-client/src/test/java/org/eclipse/jetty/client/InsufficientThreadsDetectionTest.java rename jetty-util/src/main/java/org/eclipse/jetty/util/thread/{ThreadBudget.java => ThreadPoolBudget.java} (84%) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/InsufficientThreadsDetectionTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/InsufficientThreadsDetectionTest.java new file mode 100644 index 00000000000..e7dcd432e4c --- /dev/null +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/InsufficientThreadsDetectionTest.java @@ -0,0 +1,58 @@ +// +// ======================================================================== +// Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.client; + +import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP; +import org.eclipse.jetty.util.thread.QueuedThreadPool; +import org.junit.Assert; +import org.junit.Test; + +public class InsufficientThreadsDetectionTest +{ + @Test(expected = IllegalStateException.class) + public void testInsufficientThreads() throws Exception + { + QueuedThreadPool clientThreads = new QueuedThreadPool(1); + HttpClient httpClient = new HttpClient(new HttpClientTransportOverHTTP(1), null); + httpClient.setExecutor(clientThreads); + httpClient.start(); + } + + @Test + public void testInsufficientThreadsForMultipleHttpClients() throws Exception + { + QueuedThreadPool clientThreads = new QueuedThreadPool(3); + HttpClient httpClient1 = new HttpClient(new HttpClientTransportOverHTTP(1), null); + httpClient1.setExecutor(clientThreads); + httpClient1.start(); + + try + { + // Share the same thread pool with another instance. + HttpClient httpClient2 = new HttpClient(new HttpClientTransportOverHTTP(1), null); + httpClient2.setExecutor(clientThreads); + httpClient2.start(); + Assert.fail(); + } + catch (IllegalStateException expected) + { + // Expected. + } + } +} diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index 792610ea6b6..121b6c817a3 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -38,8 +38,8 @@ import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.ReservedThreadExecutor; import org.eclipse.jetty.util.thread.Scheduler; -import org.eclipse.jetty.util.thread.ThreadBudget; import org.eclipse.jetty.util.thread.ThreadPool; +import org.eclipse.jetty.util.thread.ThreadPoolBudget; import org.eclipse.jetty.util.thread.strategy.EatWhatYouKill; /** @@ -61,6 +61,7 @@ public abstract class SelectorManager extends ContainerLifeCycle implements Dump private long _connectTimeout = DEFAULT_CONNECT_TIMEOUT; private long _selectorIndex; private int _reservedThreads = -1; + private ThreadPoolBudget.Lease _lease; private static int defaultSelectors(Executor executor) { @@ -296,14 +297,13 @@ public abstract class SelectorManager extends ContainerLifeCycle implements Dump protected void doStart() throws Exception { addBean(new ReservedThreadExecutor(getExecutor(),_reservedThreads,this),true); - + _lease = ThreadPoolBudget.leaseFrom(getExecutor(), this, _selectors.length); for (int i = 0; i < _selectors.length; i++) { ManagedSelector selector = newSelector(i); _selectors[i] = selector; addBean(selector); } - super.doStart(); } @@ -324,6 +324,8 @@ public abstract class SelectorManager extends ContainerLifeCycle implements Dump super.doStop(); for (ManagedSelector selector : _selectors) removeBean(selector); + if (_lease != null) + _lease.close(); } /** diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java index e6c53e2d40a..653a2d860d6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java @@ -53,8 +53,7 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.Locker; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; import org.eclipse.jetty.util.thread.Scheduler; -import org.eclipse.jetty.util.thread.ThreadBudget; -import org.eclipse.jetty.util.thread.ThreadPool; +import org.eclipse.jetty.util.thread.ThreadPoolBudget; /** *

An abstract implementation of {@link Connector} that provides a {@link ConnectionFactory} mechanism @@ -160,8 +159,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co private String _name; private int _acceptorPriorityDelta=-2; private boolean _accepting = true; - private ThreadBudget.Lease lease; - + private ThreadPoolBudget.Lease _lease; /** * @param server The server this connector will be added to. Must not be null. @@ -276,7 +274,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co throw new IllegalStateException("No protocol factory for SSL next protocol: '" + next + "' in " + this); } - lease = ThreadBudget.leaseFrom(getExecutor(),this,_acceptors.length); + _lease = ThreadPoolBudget.leaseFrom(getExecutor(),this,_acceptors.length); super.doStart(); _stopping=new CountDownLatch(_acceptors.length); @@ -312,8 +310,8 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co @Override protected void doStop() throws Exception { - if (lease!=null) - lease.close(); + if (_lease!=null) + _lease.close(); // Tell the acceptors we are stopping interruptAcceptors(); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java index adce494570b..7fe301f8f0c 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/InsufficientThreadsDetectionTest.java @@ -21,8 +21,8 @@ package org.eclipse.jetty.server; import org.eclipse.jetty.toolchain.test.AdvancedRunner; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.thread.QueuedThreadPool; -import org.eclipse.jetty.util.thread.ThreadBudget; import org.eclipse.jetty.util.thread.ThreadPool; +import org.eclipse.jetty.util.thread.ThreadPoolBudget; import org.junit.After; import org.junit.Assert; import org.junit.Test; @@ -61,7 +61,7 @@ public class InsufficientThreadsDetectionTest } catch(IllegalStateException e) { - Log.getLogger(ThreadBudget.class).warn(e.toString()); + Log.getLogger(ThreadPoolBudget.class).warn(e.toString()); } } @@ -107,7 +107,7 @@ public class InsufficientThreadsDetectionTest } catch(IllegalStateException e) { - Log.getLogger(ThreadBudget.class).warn(e.toString()); + Log.getLogger(ThreadPoolBudget.class).warn(e.toString()); } } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index 2606e8c291b..92b6204bc14 100755 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -65,7 +65,7 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo private boolean _daemon = false; private boolean _detailedDump = false; private int _lowThreadsThreshold = 1; - private ThreadBudget _budget; + private ThreadPoolBudget _budget; public QueuedThreadPool() { @@ -106,23 +106,20 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo } _jobs=queue; _threadGroup=threadGroup; - _budget=new ThreadBudget(this); + _budget=new ThreadPoolBudget(this); } @Override - public ThreadBudget getThreadBudget() + public ThreadPoolBudget getThreadPoolBudget() { return _budget; } - public void setThreadBudget(ThreadBudget budget) + public void setThreadPoolBudget(ThreadPoolBudget budget) { if (budget!=null && budget.getSizedThreadPool()!=this) throw new IllegalArgumentException(); - synchronized (this) - { - _budget = budget; - } + _budget = budget; } @Override diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java index d58c69b2806..0c8a2d9b6ee 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java @@ -64,7 +64,7 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo private final AtomicInteger _size = new AtomicInteger(); private final AtomicInteger _pending = new AtomicInteger(); - private ThreadBudget.Lease _lease; + private ThreadPoolBudget.Lease _lease; private Object _owner; private long _idleTime = 1L; private TimeUnit _idleTimeUnit = TimeUnit.MINUTES; @@ -168,7 +168,7 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements Executo @Override public void doStart() throws Exception { - _lease = ThreadBudget.leaseFrom(getExecutor(),this,_capacity); + _lease = ThreadPoolBudget.leaseFrom(getExecutor(),this,_capacity); super.doStart(); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java index 2bfafe13a80..ecc61bd87c3 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPool.java @@ -71,8 +71,7 @@ public interface ThreadPool extends Executor int getMaxThreads(); void setMinThreads(int threads); void setMaxThreads(int threads); - - default ThreadBudget getThreadBudget() + default ThreadPoolBudget getThreadPoolBudget() { return null; } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadBudget.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPoolBudget.java similarity index 84% rename from jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadBudget.java rename to jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPoolBudget.java index 1d9d0033007..e2ed8069a59 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadBudget.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ThreadPoolBudget.java @@ -31,11 +31,11 @@ import org.eclipse.jetty.util.log.Logger; /** *

A budget of required thread usage, used to warn or error for insufficient configured threads.

* - * @see ThreadPool.SizedThreadPool#getThreadBudget() + * @see ThreadPool.SizedThreadPool#getThreadPoolBudget() */ -public class ThreadBudget +public class ThreadPoolBudget { - static final Logger LOG = Log.getLogger(ThreadBudget.class); + static final Logger LOG = Log.getLogger(ThreadPoolBudget.class); public interface Lease extends Closeable { @@ -92,10 +92,10 @@ public class ThreadBudget final int warnAt; /** - * Construct a bedget for a SizedThreadPool, with the warning level set by heuristic. + * Construct a budget for a SizedThreadPool, with the warning level set by heuristic. * @param pool The pool to budget thread allocation for. */ - public ThreadBudget(ThreadPool.SizedThreadPool pool) + public ThreadPoolBudget(ThreadPool.SizedThreadPool pool) { this(pool,Runtime.getRuntime().availableProcessors()); } @@ -104,7 +104,7 @@ public class ThreadBudget * @param pool The pool to budget thread allocation for. * @param warnAt The level of free threads at which a warning is generated. */ - public ThreadBudget(ThreadPool.SizedThreadPool pool, int warnAt) + public ThreadPoolBudget(ThreadPool.SizedThreadPool pool, int warnAt) { this.pool = pool; this.warnAt = warnAt; @@ -139,20 +139,20 @@ public class ThreadBudget int required = allocations.stream() .mapToInt(Lease::getThreads) .sum(); - int maximum = pool.getMaxThreads(); + int actual = maximum - required; - if (required>=maximum) + if (actual <= 0) { infoOnLeases(); - throw new IllegalStateException(String.format("Insuffient configured threads: required=%d < max=%d for %s", required, maximum, pool)); + throw new IllegalStateException(String.format("Insufficient configured threads: required=%d < max=%d for %s", required, maximum, pool)); } - if ((maximum-required) < warnAt) + if (actual < warnAt) { infoOnLeases(); if (warned.compareAndSet(false,true)) - LOG.warn("Low configured threads: ( max={} - required={} ) < warnAt={} for {}", maximum, required, warnAt, pool); + LOG.warn("Low configured threads: (max={} - required={})={} < warnAt={} for {}", maximum, required, actual, warnAt, pool); } } @@ -169,7 +169,7 @@ public class ThreadBudget { if (executor instanceof ThreadPool.SizedThreadPool) { - ThreadBudget budget = ((ThreadPool.SizedThreadPool)executor).getThreadBudget(); + ThreadPoolBudget budget = ((ThreadPool.SizedThreadPool)executor).getThreadPoolBudget(); if (budget!=null) return budget.leaseTo(leasee,threads); } From 807233771df0c4a7771f8325f225e857cb27852f Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 2 Oct 2017 17:17:50 +0200 Subject: [PATCH 13/16] Removed unnecessary field _maxSize. --- .../main/java/org/eclipse/jetty/io/MappedByteBufferPool.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/MappedByteBufferPool.java b/jetty-io/src/main/java/org/eclipse/jetty/io/MappedByteBufferPool.java index 0bc2b382d4a..fa4cbd03876 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/MappedByteBufferPool.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/MappedByteBufferPool.java @@ -31,7 +31,6 @@ public class MappedByteBufferPool implements ByteBufferPool private final ConcurrentMap directBuffers = new ConcurrentHashMap<>(); private final ConcurrentMap heapBuffers = new ConcurrentHashMap<>(); private final int _factor; - private final int _maxQueue; private final Function _newBucket; public MappedByteBufferPool() @@ -52,8 +51,7 @@ public class MappedByteBufferPool implements ByteBufferPool public MappedByteBufferPool(int factor,int maxQueue,Function newBucket) { _factor = factor<=0?1024:factor; - _maxQueue = maxQueue; - _newBucket = newBucket!=null?newBucket:i->new Bucket(this,i*_factor,_maxQueue); + _newBucket = newBucket!=null?newBucket:i->new Bucket(this,i*_factor,maxQueue); } @Override From 2b43e668a3be68cb7716875a44d4758659ad8960 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 2 Oct 2017 17:18:10 +0200 Subject: [PATCH 14/16] Cosmetics. --- .../jetty/io/MappedByteBufferPoolTest.java | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/MappedByteBufferPoolTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/MappedByteBufferPoolTest.java index c0c5b46ca7d..86be6de1b1d 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/MappedByteBufferPoolTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/MappedByteBufferPoolTest.java @@ -18,14 +18,6 @@ package org.eclipse.jetty.io; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.nio.ByteBuffer; import java.util.concurrent.ConcurrentMap; @@ -34,6 +26,14 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.StringUtil; import org.junit.Test; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + public class MappedByteBufferPoolTest { @Test @@ -93,8 +93,10 @@ public class MappedByteBufferPoolTest } /** - * In a scenario where MappedByteBufferPool is being used improperly, such as releasing a buffer that wasn't created/acquired by the MappedByteBufferPool, - * an assertion is tested for. + * In a scenario where MappedByteBufferPool is being used improperly, + * such as releasing a buffer that wasn't created/acquired by the + * MappedByteBufferPool, an assertion is tested for. + * * @throws Exception test failure */ @Test @@ -133,8 +135,6 @@ public class MappedByteBufferPoolTest buffer = pool.acquire(1024,false); assertThat(BufferUtil.toDetailString(buffer),containsString("@T00000002")); } - - @Test public void testMaxQueue() throws Exception @@ -157,6 +157,5 @@ public class MappedByteBufferPoolTest bufferPool.release(buffer3); assertEquals(2, bucket.size()); - } } From c2bcdd5ed48612e6a68239c1045ddb0375db7e95 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 2 Oct 2017 17:35:33 +0200 Subject: [PATCH 15/16] Fixes #1509 - Review GZIPContentDecoder buffer pooling. GZIPContentDecoder returns to applications pooled buffers that may appear as leaked, but they are not. Fixed the test to avoid failures or exception stack traces. --- .../server/AbstractHttpClientServerTest.java | 16 +++++++++++----- .../jetty/fcgi/server/HttpClientTest.java | 6 ++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java b/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java index 58452512ccd..71d1d38b672 100644 --- a/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java +++ b/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/AbstractHttpClientServerTest.java @@ -25,6 +25,7 @@ import org.eclipse.jetty.client.HttpClientTransport; import org.eclipse.jetty.client.LeakTrackingConnectionPool; import org.eclipse.jetty.fcgi.client.http.HttpClientTransportOverFCGI; import org.eclipse.jetty.http.HttpScheme; +import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.LeakTrackingByteBufferPool; import org.eclipse.jetty.io.MappedByteBufferPool; import org.eclipse.jetty.server.Handler; @@ -45,7 +46,7 @@ public abstract class AbstractHttpClientServerTest @Rule public final TestTracker tracker = new TestTracker(); private LeakTrackingByteBufferPool serverBufferPool; - private LeakTrackingByteBufferPool clientBufferPool; + protected ByteBufferPool clientBufferPool; private final AtomicLong connectionLeaks = new AtomicLong(); protected Server server; protected ServerConnector connector; @@ -80,7 +81,8 @@ public abstract class AbstractHttpClientServerTest }); client = new HttpClient(transport, null); client.setExecutor(executor); - clientBufferPool = new LeakTrackingByteBufferPool(new MappedByteBufferPool.Tagged()); + if (clientBufferPool == null) + clientBufferPool = new LeakTrackingByteBufferPool(new MappedByteBufferPool.Tagged()); client.setByteBufferPool(clientBufferPool); client.start(); } @@ -94,9 +96,13 @@ public abstract class AbstractHttpClientServerTest assertThat("Server BufferPool - leaked releases", serverBufferPool.getLeakedReleases(), Matchers.is(0L)); assertThat("Server BufferPool - unreleased", serverBufferPool.getLeakedResources(), Matchers.is(0L)); - assertThat("Client BufferPool - leaked acquires", clientBufferPool.getLeakedAcquires(), Matchers.is(0L)); - assertThat("Client BufferPool - leaked releases", clientBufferPool.getLeakedReleases(), Matchers.is(0L)); - assertThat("Client BufferPool - unreleased", clientBufferPool.getLeakedResources(), Matchers.is(0L)); + if (clientBufferPool instanceof LeakTrackingByteBufferPool) + { + LeakTrackingByteBufferPool pool = (LeakTrackingByteBufferPool)clientBufferPool; + assertThat("Client BufferPool - leaked acquires", pool.getLeakedAcquires(), Matchers.is(0L)); + assertThat("Client BufferPool - leaked releases", pool.getLeakedReleases(), Matchers.is(0L)); + assertThat("Client BufferPool - unreleased", pool.getLeakedResources(), Matchers.is(0L)); + } assertThat("Connection Leaks", connectionLeaks.get(), Matchers.is(0L)); diff --git a/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java b/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java index 972c7dc06e0..6baf3b69242 100644 --- a/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java +++ b/jetty-fcgi/fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java @@ -46,6 +46,7 @@ import org.eclipse.jetty.client.util.BytesContentProvider; import org.eclipse.jetty.client.util.DeferredContentProvider; import org.eclipse.jetty.client.util.FutureResponseListener; import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.io.MappedByteBufferPool; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.toolchain.test.IO; import org.eclipse.jetty.toolchain.test.annotation.Slow; @@ -380,6 +381,11 @@ public class HttpClientTest extends AbstractHttpClientServerTest @Test public void testGZIPContentEncoding() throws Exception { + // GZIPContentDecoder returns to application pooled + // buffers, which is fine, but in this test they will + // appear as "leaked", so we use a normal ByteBufferPool. + clientBufferPool = new MappedByteBufferPool.Tagged(); + final byte[] data = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; start(new AbstractHandler() { From 7b93c3c1e7c92320ca34104ece0c56292a0836fb Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 3 Oct 2017 10:16:09 +1100 Subject: [PATCH 16/16] Issue #1863 --- .../eclipse/jetty/annotations/TestAnnotationConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java index d56e31aa16a..4eba9722a40 100644 --- a/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java +++ b/jetty-annotations/src/test/java/org/eclipse/jetty/annotations/TestAnnotationConfiguration.java @@ -195,7 +195,7 @@ public class TestAnnotationConfiguration String dir = MavenTestingUtils.getTargetTestingDir("getFragmentFromJar").getAbsolutePath(); File file = new File(dir); file=new File(file.getCanonicalPath()); - URL url=file.toURL(); + URL url=file.toURI().toURL(); Resource jar1 = Resource.newResource(url+"file.jar");