diff --git a/VERSION.txt b/VERSION.txt index 8e06c966a89..04851174b5c 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1,6 +1,31 @@ jetty-10.0.0-SNAPSHOT -jetty-10.0.0.beta0 - 27 May 2020 +jetty-9.4.31.v20200723 - 23 July 2020 + + 1100 JSR356 Encoder#init is not called when created on demand + + 4736 Update Import-Package version start ranges + + 4890 JettyClient behavior when SETTINGS_HEADER_TABLE_SIZE is set to 0 in + SETTINGS Frame. + + 4904 WebsocketClient creates more connections than needed + + 4965 WINDOW_UPDATE for locally failed stream should not close the HTTP/2 + session + + 4967 Possible buffer corruption in HTTP/2 session failures + + 4971 Simplify Connection.upgradeFrom()/upgradeTo() + + 4976 HttpClient async content throws NPE in DEBUG log + + 4981 Incorrect example for TryFilesFilter API docs + + 4985 Fix NPE related to use of Attributes.Wrapper getAttributeNameSet() + + 4989 Prevent parsing of module-info.class in OSGi bundles + + 5000 NPE from Server.dump of FilterMapping + + 5013 Bundle-ClassPath and lib place on WEB-INF/lib make classpath duplicate + + 5018 WebSocketClient connect / upgrade timeout not configurable + + 5019 Automatically hot-reload SSL certificates if keystore file changed + + 5020 LifeCycle.Listener not called for Filter/Servlet/Listener lifecycle + events + + 5025 dispatcher.include() with welcome files lead to stack overflow error + + 5053 CWE-331 in DigestAuthentication class + + 5057 `javax.servlet.include.context_path` attribute on root context. should + be empty string, but is `"/"` + + 5064 NotSerializableException for OpenIdConfiguration + + 5069 HttpClientTimeoutTests can occasionally fail due to unreachable network jetty-9.4.30.v20200611 - 11 June 2020 + 4776 Incorrect path matching for WebSocket using PathMappings @@ -48,361 +73,6 @@ jetty-9.4.29.v20200521 - 21 May 2020 + 4895 AbstractSessionCache.setFlushOnResponseCommit(true) can write an invalid session to the backing store -jetty-10.0.0.alpha1 - 26 November 2019 - + 97 Permanent UnavailableException thrown during servlet request handling - should cause servlet destroy - + 137 Support OAuth - + 155 No way to set keystore for JSR 356 websocket clients, needed for SSL - client authentication - + 250 Implement HTTP CONNECT for HTTP/2 - + 995 UrlEncoded.encodeString should skip more characters - + 1036 Allow easy configuration of Scheduler-Threads and name them more - appropriate - + 1485 Add systemd service file - + 1743 Refactor jetty maven plugin goals to be more orthogonal - + 2266 Jetty maven plugin reload is triggered each time the - `scanIntervalSeconds` pass - + 2340 Remove raw ServletHandler usage examples from documentation - + 2429 Review HttpClient backpressure semantic - + 2578 Use addEventListener(EventListener listener) - + 2709 current default for headerCacheSize is not large enough for many - requests - + 2815 hpack fields are opaque octets - + 3040 Allow RFC6265 Cookies to include optional SameSite attribute - + 3083 The ini-template for jetty.console-capture.dir does not match the - default value - + 3106 Websocket connection stats and request stats - + 3558 Error notifications can be received after a successful websocket close - + 3601 HTTP2 stall on reset streams - + 3705 Review ClientUpgradeRequest exception handling - + 3734 websocket suspend when input closed - + 3747 Make Jetty Demo work with JPMS - + 3787 Jetty client sometimes returns EOFException instead of - SSLHandshakeException on certificate errors. - + 3804 Weld/CDI XML backwards compat? - + 3806 Error Page handling Async race with ProxyServlet - + 3822 trustAll will not work on some servers - + 3829 Avoid sending empty trailer frames for http/2 responses - + 3840 Byte-range request performance problems with large files - + 3856 Different behaviour with maxFormContentSize=0 if Content-Length header - is present/missing - + 3863 Enforce use of SNI - + 3869 Update to ASM 7.2 for jdk 13 - + 3872 Review exposure of JavaxWebSocketServletContainerInitializer - + 3876 WebSocketPartialListener is only called for initial frames, not for - continuation frames - + 3884 @WebSocket without @OnWebSocketMessage handler fails when receiving a - continuation frame - + 3888 BufferUtil.toBuffer(Resource resource,boolean direct) does not like - large (4G+) Resources - + 3906 Fix for #3840 breaks Path encapsulation in PathResource - + 3913 Clustered HttpSession IllegalStateException: Invalid for read - + 3929 Deadlock between new HTTP2Connection() and Server.stop() - + 3936 Race condition when modifying session + sendRedirect() - + 3940 Double initialization of Log - + 3951 Consider adding demand API to HTTP/2 - + 3952 Server configuration for direct/heap ByteBuffers - + 3956 Remove and warn on use of illegal HTTP/2 response headers - + 3957 CustomRequestLog bad usage of MethodHandles.lookup() - + 3960 Fix HttpConfiguration copy constructor - + 3964 Improve efficiency of listeners - + 3968 WebSocket sporadic ReadPendingException using suspend/resume - + 3969 X-Forwarded-Port header customization isn't possible - + 3978 HTTP/2 fixes for robustly handling abnormal traffic and resource - exhaustion - + 3983 JarFileResource incorrectly lists the contents of directories with - spaces - + 3985 Improve lenient Cookie parsing - + 3989 Inform custom ManagedSelector of dead selector via optional - onFailedSelect() - + 4000 Add SameFileAliasChecker to help with FileSystem static file access - normalization on Mac and Windows - + 4003 Quickstart broken in jetty-10 - + 4007 Getting NullPointerException while trying to run jetty start.run on - Windows - + 4009 ServletContextHandler setSecurityHandler broke handler chain - + 4020 Revert WebSocket ExtensionFactory change to interface - + 4022 Servlet which is added by ServletRegistration can't be started - + 4025 Provide more write-through behaviours for DefaultSessionCache - + 4027 Ensure AbstractSessionDataStore cannot be used unless it is started - + 4033 Ignore bad percent encodings in paths during - URIUtil.equalsIgnoreEncodings() - + 4047 Gracefully stopped Jetty not flushing all response data - + 4048 Multiple values in X-Forwarded-Port throw NumberFormatException - + 4057 NullPointerException in o.e.j.h.HttpFields - + 4058 Review Locker - + 4064 java.lang.NullPointerException initializing embedded servlet - + 4075 Do not fail on servlet-mapping with url-pattern /On* - + 4076 Restarting quickstarted webapp throws IllegalStateException: - ServletContainerInitializersStarter already exists - + 4082 Debug logging causes NullPointerException in client - + 4084 Use of HttpConfiguration.setBlockingTimeout(long) in jetty.xml produces - warning on jetty-home startup - + 4096 Thread in ReservedThreadExecutor does not exit when stopped - + 4104 Frames are sent through ExtensionStack even if WebSocket Session is - closed - + 4105 QueuedThreadPool increased thread usage and no idle thread decay - + 4113 HttpClient fails with JDK 13 and TLS 1.3 - + 4115 Drop HTTP/2 pseudo headers - + 4121 QueuedThreadPool should support ThreadFactory behaviors - + 4122 QueuedThreadPool should reset thread interrupted on failed run - + 4124 Run websocket autobahn tests with jetty and javax apis instead of just - with core. - + 4128 OpenIdCredentials can't decode JWT ID token - + 4132 Should be possible to use OIDC without metadata - + 4138 OpenID module should use HttpClient instead of HttpURLConnection - + 4141 ClassCastException with non-async Servlet + async Filter + - HttpServletRequestWrapper - + 4142 Configurable HTTP/2 RateControl - + 4144 Naked cast to Request should be avoided - + 4150 Module org.eclipse.jetty.alpn.client not found, required by - org.eclipse.jetty.proxy - + 4152 WebSocket autoFragment does not fragment based on maxFrameSize - + 4156 IllegalStateException when forwarding to jsp with new session - + 4161 Regression: EofException: request lifecycle violation - + 4170 Client-side alias selection based on SSLEngine - + 4173 NullPointerException warning in log from WebInfConfiguration after - upgrade - + 4174 ConcurrentModificationException when stopping jetty:run-war - + 4176 Should not set header if sendError has been called - + 4177 Configure HTTP proxy with SslContextFactory - + 4179 Improve HttpChannel$SendCallback references for GC - + 4183 Jetty considers bootstrap injected class to be a "server class" - + 4188 Spin in HttpOutput.close - + 4190 Jetty hangs after thread blocked in SharedBlockingCallback.block() - called by HttpOutput.close - + 4191 Increase GzipHandler minGzipSize default value - + 4193 InetAccessHandler - new includeConnectors/excludeConnectors not quite - correct anymore - + 4201 Throw SSLHandshakeException in case of TLS handshake failures - + 4203 Some Transfer-Encoding and Content-Length combinations do not result in - expected 400 Bad Request - + 4204 Transfer-Encoding behavior does not follow RFC7230 - + 4208 304 response with Content-Length fails, not conform to RFC7230 - + 4209 Unused TLS connection is not closed in Java 11 - + 4217 SslConnection.DecryptedEnpoint.flush eternal busy loop - + 4222 Major/Minor Version wrong (jetty 10 is servlet 4) - + 4227 First authorization request produced by OIDC module fails due to - inclusion of sessionid - + 4236 clean up redirect code calculation for OpenIdAuthenticator - + 4237 simplify openid module configuration - + 4240 CGI form post results in 500 response if no character encoding - + 4243 ErrorHandler produces invalid json error response - + 4247 Cookie security attributes are going to mandated by Google Chrome - + 4248 Websocket client UpgradeListener never reports success - + 4251 Http 2.0 clients cannot upgrade protocol - + 4258 RateControl should be per-connection - + 4264 Spring Boot BasicErrorController no longer invoked - + 4265 HttpChannel SEND_ERROR should use ErrorHandler.doError() - + 4277 Reading streamed gzipped body never terminates - + 4279 Regression: ResponseWriter#close blocks indefinitely - + 4282 Review HttpParser handling in case of no content - + 4283 Wrong package for OpenJDK8ClientALPNProcessor - + 4284 Possible NullPointerException in Main.java when stopped from command - line - + 4287 Move getUriLastPathSegment(URI uri) to URIUtil - + 4296 Unable to create WebSocket connect if the query string of the URL has % - symbol. - + 4301 Demand beforeContent is not forwarded - + 4305 Jetty server ALPN shall alert fatal no_application_protocol if no - client application protocol is supported - + 4325 Deprecate SniX509ExtendedKeyManager constructor without - SslContextFactory$Server - + 4334 Better test ErrorHandler changes - + 4342 OpenID module cannot create HttpClient in Jetty 10 - -jetty-10.0.0-alpha0 - 11 July 2019 - + 113 Add support for NCSA Extended Log File Format - + 114 Bring back overlay deployer - + 132 ClientConnector abstraction - + 207 Support javax.websocket version 1.1 - + 215 Add Conscrypt for native ALPN/TLS/SSL - + 300 Implement Deflater / Inflater Object Pool - + 482 jetty-osgi] The CCL while parsing the xml files should be set to a - combination of Jetty and Bundle-Classloader - + 592 Support no-value Host header in HttpParser - + 632 JMX tests rely on fixed port - + 675 Slf4jLog.ignore() should produce at DEBUG level - + 676 JavaUtilLog.ignore() should produce at DEBUG level - + 677 Logging of .ignore() should indicate that it was an "Ignored Exception" - + 746 Implement Servlet 4.0 Request.getMappings - + 801 Jetty respond with status 200 instead of 304 while using Servlet 4.0 - PushBuilder - + 809 NPE in WebInfConfiguration for webapp deploy in osgi - + 987 Can GzipHandler check if .gz file exists only by some paths? - + 1135 Avoid allocations from Method.getParameterTypes() if possible - + 1200 Use PathWatcher in DeploymentManager - + 1350 Dynamic selection of the transport to use based on ALPN on the client - side - + 1368 Need to support KeyStore/TrustStore with null passwords - + 1384 Expose StatisticsServlet to webapp - + 1468 Configure PKIX Revocation Checker for SslContextFactory - + 1485 Add systemd service file - + 1498 Add JRTResource to support future Java 9 classloader behaviors - + 1499 ClasspathPattern needs MODULE ruleset to support future Java 9 - classloader behaviors - + 1503 IPv6 address needs normalization (without brackets) in - ForwardedRequestCustomizer - + 1551 Move CookieCutter to jetty-http - + 1571 Support Hazelcast session management in 9.4 - + 1574 TypeUtilTest#testGetLocationOfClass is user settings dependant - + 1591 JDBCSessionDataStore doesn't work with root context on Oracle DB - + 1592 CompressedContentFormat.tagEquals() - incorrect comparison of entity - tag hashes - + 1595 HTTP/2: Avoid sending unnecessary stream WINDOW_UPDATE frames - + 1599 WebSocketCloseTest fails - + 1600 Update jndi.mod and plus.mod - + 1603 WebSocketServerFactory NPE in toString() - + 1604 WebSocketContainer stop needs improvement - + 1605 ContainerProvider.getWebSocketContainer() behavior is not to spec - + 1615 Password defaults in jetty-ssl-context.xml should be removed - + 1618 AsyncContext.dispatch() does not use raw/encoded URI - + 1622 HeaderFilter doesn't work if the response has been committed - + 1623 JettyRunMojo use dependencies from reactor (outputdirectory) - + 1625 Support new IANA declared Websocket Close Status Codes - + 1637 Thread per connection retained in HTTP/2 - + 1638 Add it test for Maven Plugin - + 1642 Using RewriteHandler with AsyncContext.dispatch() and - HttpServletRequestWrapper not possible - + 1643 ProxyServlet always uses default number of selector threads - - constructor should allow to overwrite the default. - + 1645 NotSerializableException: DoSFilter when using Non-Clustered Session - Management: File System - + 1656 Improve configurability of ConnectionPools - + 1671 Asymmetric usage of trailers in MetaData.Request - + 1675 Session id should not be logged with INFO level in AbstractSessionCache - + 1676 Remove Deprecated classes & methods - + 1679 DeploymentManagerMBean not usable through JMX - + 1682 Jetty-WarFragmentFolderPath directive has no effect in eclipse runtime - mode except for the first launch - + 1692 Annotation scanning should ignore `module-info.class` files - + 1698 Missing WWW-Authenticate from SpnegoAuthenticator when other - Authorization header provided - + 1746 Remove LICENSE-CONTRIBUTOR? - + 1836 Migrate Locker implementation to JVM ReentrantLock implementation - + 1838 Servlet 4.0.0 artifact now available on central.maven.org - + 1852 Fix quickstart generation for servlet 4.0 - + 1898 Request.getCookie() should ignore invalid cookies - + 1956 Store and report build information of Jetty - + 1977 jetty-http-spi tests fail with java9 - + 2061 WebSocket hangs in blockingWrite - + 2075 Deprecating MultiException - + 2095 Remove FastCGI multiplexing - + 2103 Server should open connectors early in start sequence - + 2108 Update licence headers and plugin for 2018 - + 2140 Infinispan and hazelcast changes to scavenge zombie expired sessions - + 2172 Support javax.websocket 1.1 - + 2175 Refactor WebSocket close handling - + 2191 JPMS Support - + 2431 Upgrade to Junit 5 - + 2868 Adding SPNEGO authentication support for Jetty Client - + 2901 Introduce HttpConnectionUpgrader as a conversation component in - HttpClient - + 2909 Remove B64Code - + 2948 Require JDK 11 for Jetty 10.x - + 2978 Add module-info.java to relevant Jetty modules - + 2983 Jetty 10 Configuration abstraction - + 2985 Jetty 10 Configuration replacement algorithm incorrect - + 2996 ContextHandler.setDefaultContextPath() not implemented for quickstart - + 3009 Update Jetty 10 to use non-LEGACY Compliance Modes - + 3010 Move old MultiPart parsing implementation to jetty-http - + 3011 Move HttpCompliance to HttpConfiguration - + 3012 Refactor HttpCompliance and HttpComplianceSection to be friendlier to - customization - + 3129 javax-websocket-common pom.xml is wrong - + 3139 NPE on - WebSocketServerContainerInitializer.configureContext(ServletContextHandler) - + 3154 Add support for javax.net.ssl.HostnameVerifier to HttpClient - + 3159 WebSocket permessage-deflate RSV1 validity check - + 3162 Use Jetty specific Servlet API jar - + 3165 Review javax-websocket-server tests - + 3166 Run of autobahn websocket tests on CI - + 3167 JavaxWebSocketServerContainerInitializer always creates a HttpClient - + 3170 WebSocket proxy PoC - + 3182 Restore websocket example files - + 3186 Jetty maven plugin - javax.annotation.jar picked up from jetty plugin - rather than from applications classpath - + 3197 Use jetty specific websocket API jar - + 3213 MetaInfConfigurationTest tests disabled in jetty-10.0.x - + 3216 Autobahn WebSocketServer failures in jetty 10 - + 3225 Response.sendError should not set reason - + 3246 javax-websocket-tests exception stacktraces - + 3249 Update to apache jasper 9.0.14 for jetty-10 - + 3274 OSGi versions of java.base classes in - org.apache.felix:org.osgi.foundation:jar conflicts with new rules on Java 9+ - + 3279 WebSocket write may hang forever - + 3288 Correct websocket artifactIds on jetty-10.0.x - + 3290 async websocket onOpen, onError and onClose in 10.0.x - + 3298 Review jetty-10 websocket CompletableFuture usage - + 3303 Update to jakarta ee javax artifacts for jetty-10 - + 3308 Remove deprecated methods from sessions - + 3320 Review Jetty 10 module-info.java - + 3333 Jetty 10 standalone cannot start on the module-path - + 3340 Update PushCacheFilter to use Servlet 4.0 APIs - + 3341 XmlBasedHttpClientProvider in Jetty 10 - + 3351 Restructure jetty-unixsocket module - + 3374 JSR356 RemoteEndpoint.Async.setSendTimeout() logic invalid in Jetty - 10.0.x - + 3379 Tracking of WebSocket Sessions in WebSocket containers - + 3380 WebSocket should support jetty-io Connection.Listener - + 3382 Jetty WebSocket Session.suspend() not implemented - + 3399 XmlConfiguration jetty.webapps.uri is the uri of the webapp not the - parent dir - + 3406 restore and fix jetty websocket tests in jetty 10 - + 3412 problems with jetty 10 WebSocket session customizer - + 3446 allow jetty WebSockets to be upgraded using WebSocketUpgradeFilter in - jetty-10 - + 3453 Removing moved Extension classes from jetty-websocket-api - + 3458 ensure users of the jetty-websocket-api do not have to see - websocket-core classes - + 3462 client validation of websocket upgrade response - + 3465 websocket negotiation of extension configuration parameters - + 3479 review and cleanup of jetty-websocket-api in jetty-10 - + 3484 ClassCastException when using websocket-core classes in - websocket-servlet - + 3494 flaky tests in jetty-websocket-tests ClientCloseTest - + 3564 Update jetty-10.0.x to apache jsp 9.0.19 - + 3608 Reply with 400 Bad request to malformed WebSocket handshake - + 3616 Backport WebSocket SessionTracker from Jetty 10 - + 3648 javax.websocket client container incorrectly creates Server - SslContextFactory - + 3661 JettyWebSocketServerContainer exposes websocket common classes - + 3666 WebSocket - Handling sent 1009 close frame - + 3696 Unwrap JavaxWebSocketClientContainer.connectToServer() exceptions - + 3698 Missing WebSocket ServerContainer after server restart - + 3700 stackoverflow in WebAppClassLoaderUrlStreamTest - + 3705 Review ClientUpgradeRequest exception handling - + 3708 Swap various java.lang.String replace() methods for better performant - ones - + 3712 change maxIdleTime to idleTimeout in jetty-10 websockets - + 3719 Clean up jetty-10 modules - + 3726 Remove OSGi export uses of servlet-api from jetty-util - + 3731 Add testing of CDI behaviors - + 3736 NPE from WebAppClassLoader during CDI - + 3746 ClassCastException in WriteFlusher.java - IdleState cannot be cast to - FailedState - + 3749 Memory leak while processing AsyncListener annotations - + 3751 Modern Configure DTD / FPI is used inconsistently - + 3755 ServerWithAnnotations doesn't do anything - + 3758 Avoid sending empty trailer frames for http/2 requests - + 3762 WebSocketConnectionStatsTest test uses port 8080 - + 3782 X-Forwarded-Port overrides X-Forwarded-For - + 3786 ALPN support for Java 14 - + 3789 XmlConfiguration set from property - + 3798 ClasspathPattern match method throws NPE. URI can be null - + 3799 Programmatically added listeners from - ServletContextListener.contextInitialzed() are not called - + 3804 Weld/CDI XML backwards compat? - + 3805 XmlConfiguration odd behavior for numbers - + 3809 sending WebSocket close frame with error StatusCode does not do a hard - close (Jetty-10) - + 3815 PropertyFileLoginModule adds user principle as a role - + 3835 WebSocketSession are not being stopped properly - + 3839 JavaxWebSocketServletContainerInitializer fails - + 3840 Byte-range request performance problems with large files - + 3849 ClosedChannelException from jetty-test-webapp javax websocket chat - example - jetty-9.4.28.v20200408 - 08 April 2020 + 847 Setting async timeout on WebSocketClient does not seem to timeout writes + 2896 Wrong Certificate Selected When Using Multiple Virtual Host Names in diff --git a/build-resources/pom.xml b/build-resources/pom.xml index 89f78175b64..6e6de0f7032 100644 --- a/build-resources/pom.xml +++ b/build-resources/pom.xml @@ -67,6 +67,7 @@ maven-deploy-plugin 2.8.2 + true diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java index 8ffb9e703dc..ad63674df48 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java @@ -18,74 +18,121 @@ package org.eclipse.jetty.client; +import java.io.IOException; +import java.util.ArrayDeque; import java.util.Collection; -import java.util.concurrent.atomic.AtomicBoolean; +import java.util.Queue; +import java.util.concurrent.CompletableFuture; +import java.util.stream.Collectors; import org.eclipse.jetty.client.api.Connection; -import org.eclipse.jetty.util.AtomicBiInteger; +import org.eclipse.jetty.util.Attachable; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.Pool; import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.Dumpable; +import org.eclipse.jetty.util.thread.Sweeper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static java.util.stream.Collectors.toCollection; + @ManagedObject -public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable +public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable, Sweeper.Sweepable { private static final Logger LOG = LoggerFactory.getLogger(AbstractConnectionPool.class); - /** - * The connectionCount encodes both the total connections plus the pending connection counts, so both can be atomically changed. - * The bottom 32 bits represent the total connections and the top 32 bits represent the pending connections. - */ - private final AtomicBiInteger connections = new AtomicBiInteger(); - private final AtomicBoolean closed = new AtomicBoolean(); private final HttpDestination destination; - private final int maxConnections; private final Callback requester; + private final Pool pool; - protected AbstractConnectionPool(HttpDestination destination, int maxConnections, Callback requester) + protected AbstractConnectionPool(HttpDestination destination, int maxConnections, boolean cache, Callback requester) { this.destination = destination; - this.maxConnections = maxConnections; this.requester = requester; + @SuppressWarnings("unchecked") + Pool pool = destination.getBean(Pool.class); + if (pool == null) + { + pool = new Pool<>(maxConnections, cache ? 1 : 0); + destination.addBean(pool); + } + this.pool = pool; } - protected HttpDestination getHttpDestination() + @Override + public CompletableFuture preCreateConnections(int connectionCount) { - return destination; + CompletableFuture[] futures = new CompletableFuture[connectionCount]; + for (int i = 0; i < connectionCount; i++) + { + futures[i] = tryCreateReturningFuture(pool.getMaxEntries()); + } + return CompletableFuture.allOf(futures); + } + + protected int getMaxMultiplex() + { + return pool.getMaxMultiplex(); + } + + protected void setMaxMultiplex(int maxMultiplex) + { + pool.setMaxMultiplex(maxMultiplex); + } + + protected int getMaxUsageCount() + { + return pool.getMaxUsageCount(); + } + + protected void setMaxUsageCount(int maxUsageCount) + { + pool.setMaxUsageCount(maxUsageCount); + } + + @ManagedAttribute(value = "The number of active connections", readonly = true) + public int getActiveConnectionCount() + { + return pool.getInUseConnectionCount(); + } + + @ManagedAttribute(value = "The number of idle connections", readonly = true) + public int getIdleConnectionCount() + { + return pool.getIdleConnectionCount(); } @ManagedAttribute(value = "The max number of connections", readonly = true) public int getMaxConnectionCount() { - return maxConnections; + return pool.getMaxEntries(); } @ManagedAttribute(value = "The number of connections", readonly = true) public int getConnectionCount() { - return connections.getLo(); + return pool.size(); } @ManagedAttribute(value = "The number of pending connections", readonly = true) public int getPendingConnectionCount() { - return connections.getHi(); + return pool.getPendingConnectionCount(); } @Override public boolean isEmpty() { - return connections.getLo() == 0; + return pool.size() == 0; } @Override public boolean isClosed() { - return closed.get(); + return pool.isClosed(); } @Override @@ -112,101 +159,175 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable */ protected void tryCreate(int maxPending) { - while (true) + tryCreateReturningFuture(maxPending); + } + + private CompletableFuture tryCreateReturningFuture(int maxPending) + { + CompletableFuture future = new CompletableFuture<>(); + + if (LOG.isDebugEnabled()) + LOG.debug("tryCreate {}/{} connections {}/{} pending", pool.size(), pool.getMaxEntries(), getPendingConnectionCount(), maxPending); + + Pool.Entry entry = pool.reserve(maxPending); + if (entry == null) { - long encoded = connections.get(); - int pending = AtomicBiInteger.getHi(encoded); - int total = AtomicBiInteger.getLo(encoded); + future.complete(null); + return future; + } - if (LOG.isDebugEnabled()) - LOG.debug("tryCreate {}/{} connections {}/{} pending", total, maxConnections, pending, maxPending); + if (LOG.isDebugEnabled()) + LOG.debug("newConnection {}/{} connections {}/{} pending", pool.size(), pool.getMaxEntries(), getPendingConnectionCount(), maxPending); - if (total >= maxConnections) - return; - - if (maxPending >= 0 && pending >= maxPending) - return; - - if (connections.compareAndSet(encoded, pending + 1, total + 1)) + destination.newConnection(new Promise<>() + { + @Override + public void succeeded(Connection connection) { if (LOG.isDebugEnabled()) - LOG.debug("newConnection {}/{} connections {}/{} pending", total + 1, maxConnections, pending + 1, maxPending); - - destination.newConnection(new Promise<>() - { - @Override - public void succeeded(Connection connection) - { - if (LOG.isDebugEnabled()) - LOG.debug("Connection {}/{} creation succeeded {}", total + 1, maxConnections, connection); - connections.add(-1, 0); - onCreated(connection); - proceed(); - } - - @Override - public void failed(Throwable x) - { - if (LOG.isDebugEnabled()) - LOG.debug("Connection " + (total + 1) + "/" + maxConnections + " creation failed", x); - connections.add(-1, -1); - requester.failed(x); - } - }); - - return; + LOG.debug("Connection {}/{} creation succeeded {}", pool.size(), pool.getMaxEntries(), connection); + adopt(entry, connection); + future.complete(null); + proceed(); } - } + + @Override + public void failed(Throwable x) + { + if (LOG.isDebugEnabled()) + LOG.debug("Connection " + pool.size() + "/" + pool.getMaxEntries() + " creation failed", x); + pool.remove(entry); + future.completeExceptionally(x); + requester.failed(x); + } + }); + return future; } @Override public boolean accept(Connection connection) { - while (true) - { - int count = connections.getLo(); - if (count >= maxConnections) - return false; - if (connections.compareAndSetLo(count, count + 1)) - return true; - } + Pool.Entry entry = pool.reserve(-1); + if (entry == null) + return false; + adopt(entry, connection); + return true; } - protected abstract void onCreated(Connection connection); - protected void proceed() { requester.succeeded(); } - protected abstract Connection activate(); - - protected Connection active(Connection connection) + private void adopt(Pool.Entry entry, Connection connection) { + if (!(connection instanceof Attachable)) + throw new IllegalArgumentException("Invalid connection object: " + connection); + Attachable attachable = (Attachable)connection; + attachable.setAttachment(entry); if (LOG.isDebugEnabled()) - LOG.debug("Connection active {}", connection); - acquired(connection); - return connection; + LOG.debug("onCreating {}", entry); + onCreated(connection); + entry.enable(connection); + idle(connection, false); } - protected void acquired(Connection connection) + protected Connection activate() + { + Pool.Entry entry = pool.acquire(); + if (LOG.isDebugEnabled()) + LOG.debug("activated '{}'", entry); + if (entry != null) + { + Connection connection = entry.getPooled(); + acquired(connection); + return connection; + } + return null; + } + + @Override + public boolean isActive(Connection connection) + { + if (!(connection instanceof Attachable)) + throw new IllegalArgumentException("Invalid connection object: " + connection); + Attachable attachable = (Attachable)connection; + @SuppressWarnings("unchecked") + Pool.Entry entry = (Pool.Entry)attachable.getAttachment(); + if (entry == null) + return false; + if (LOG.isDebugEnabled()) + LOG.debug("isActive {}", entry); + return !entry.isIdle(); + } + + @Override + public boolean release(Connection connection) + { + if (!deactivate(connection)) + return false; + released(connection); + return idle(connection, isClosed()); + } + + protected boolean deactivate(Connection connection) + { + if (!(connection instanceof Attachable)) + throw new IllegalArgumentException("Invalid connection object: " + connection); + Attachable attachable = (Attachable)connection; + @SuppressWarnings("unchecked") + Pool.Entry entry = (Pool.Entry)attachable.getAttachment(); + if (entry == null) + return true; + if (LOG.isDebugEnabled()) + LOG.debug("releasing {}", entry); + boolean reusable = pool.release(entry); + if (!reusable) + { + remove(connection); + return false; + } + return true; + } + + @Override + public boolean remove(Connection connection) + { + return remove(connection, false); + } + + protected boolean remove(Connection connection, boolean force) + { + if (!(connection instanceof Attachable)) + throw new IllegalArgumentException("Invalid connection object: " + connection); + Attachable attachable = (Attachable)connection; + @SuppressWarnings("unchecked") + Pool.Entry entry = (Pool.Entry)attachable.getAttachment(); + if (entry == null) + return false; + attachable.setAttachment(null); + if (LOG.isDebugEnabled()) + LOG.debug("removing {}", entry); + boolean removed = pool.remove(entry); + if (removed || force) + { + released(connection); + removed(connection); + } + return removed; + } + + protected void onCreated(Connection connection) { } protected boolean idle(Connection connection, boolean close) { - if (close) - { - if (LOG.isDebugEnabled()) - LOG.debug("Connection idle close {}", connection); - return false; - } - else - { - if (LOG.isDebugEnabled()) - LOG.debug("Connection idle {}", connection); - return true; - } + return !close; + } + + protected void acquired(Connection connection) + { } protected void released(Connection connection) @@ -215,28 +336,68 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable protected void removed(Connection connection) { - int pooled = connections.addAndGetLo(-1); - if (LOG.isDebugEnabled()) - LOG.debug("Connection removed {} - pooled: {}", connection, pooled); + } + + Queue getIdleConnections() + { + return pool.values().stream() + .filter(Pool.Entry::isIdle) + .filter(entry -> !entry.isClosed()) + .map(Pool.Entry::getPooled) + .collect(toCollection(ArrayDeque::new)); + } + + Collection getActiveConnections() + { + return pool.values().stream() + .filter(entry -> !entry.isIdle()) + .filter(entry -> !entry.isClosed()) + .map(Pool.Entry::getPooled) + .collect(Collectors.toList()); } @Override public void close() { - if (closed.compareAndSet(false, true)) - { - connections.set(0, 0); - } - } - - protected void close(Collection connections) - { - connections.forEach(Connection::close); + pool.close(); } @Override - public String dump() + public void dump(Appendable out, String indent) throws IOException { - return Dumpable.dump(this); + Dumpable.dumpObjects(out, indent, this); + } + + @Override + public boolean sweep() + { + pool.values().stream().filter(entry -> entry.getPooled() instanceof Sweeper.Sweepable).forEach(entry -> + { + Connection connection = entry.getPooled(); + if (((Sweeper.Sweepable)connection).sweep()) + { + boolean removed = remove(connection); + LOG.warn("Connection swept: {}{}{} from active connections{}{}", + connection, + System.lineSeparator(), + removed ? "Removed" : "Not removed", + System.lineSeparator(), + dump()); + } + }); + return false; + } + + @Override + public String toString() + { + return String.format("%s@%x[c=%d/%d/%d,a=%d,i=%d]", + getClass().getSimpleName(), + hashCode(), + getPendingConnectionCount(), + getConnectionCount(), + getMaxConnectionCount(), + getActiveConnectionCount(), + getIdleConnectionCount()); } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ConnectionPool.java index a2ad7c14263..7b79441de0d 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ConnectionPool.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.client; import java.io.Closeable; +import java.util.concurrent.CompletableFuture; import org.eclipse.jetty.client.api.Connection; @@ -27,6 +28,16 @@ import org.eclipse.jetty.client.api.Connection; */ public interface ConnectionPool extends Closeable { + /** + * Optionally pre-create up to connectionCount + * connections so they are immediately ready for use. + * @param connectionCount the number of connections to pre-start. + */ + default CompletableFuture preCreateConnections(int connectionCount) + { + return CompletableFuture.completedFuture(null); + } + /** * @param connection the connection to test * @return whether the given connection is currently in use diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/DuplexConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/DuplexConnectionPool.java index e9415af3cd5..650a337e6c2 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/DuplexConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/DuplexConnectionPool.java @@ -18,303 +18,33 @@ package org.eclipse.jetty.client; -import java.io.IOException; -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Deque; -import java.util.HashSet; -import java.util.List; -import java.util.Queue; -import java.util.Set; -import java.util.concurrent.locks.ReentrantLock; -import java.util.stream.Collectors; - -import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; -import org.eclipse.jetty.util.component.Dumpable; -import org.eclipse.jetty.util.component.DumpableCollection; -import org.eclipse.jetty.util.thread.Sweeper; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @ManagedObject -public class DuplexConnectionPool extends AbstractConnectionPool implements Sweeper.Sweepable +public class DuplexConnectionPool extends AbstractConnectionPool { - private static final Logger LOG = LoggerFactory.getLogger(DuplexConnectionPool.class); - - private final ReentrantLock lock = new ReentrantLock(); - private final Deque idleConnections; - private final Set activeConnections; - public DuplexConnectionPool(HttpDestination destination, int maxConnections, Callback requester) { - super(destination, maxConnections, requester); - this.idleConnections = new ArrayDeque<>(maxConnections); - this.activeConnections = new HashSet<>(maxConnections); + this(destination, maxConnections, true, requester); } - protected void lock() + public DuplexConnectionPool(HttpDestination destination, int maxConnections, boolean cache, Callback requester) { - lock.lock(); - } - - protected void unlock() - { - lock.unlock(); - } - - @ManagedAttribute(value = "The number of idle connections", readonly = true) - public int getIdleConnectionCount() - { - lock(); - try - { - return idleConnections.size(); - } - finally - { - unlock(); - } - } - - @ManagedAttribute(value = "The number of active connections", readonly = true) - public int getActiveConnectionCount() - { - lock(); - try - { - return activeConnections.size(); - } - finally - { - unlock(); - } - } - - public Queue getIdleConnections() - { - return idleConnections; - } - - public Collection getActiveConnections() - { - return activeConnections; + super(destination, maxConnections, cache, requester); } @Override - public boolean isActive(Connection connection) + @ManagedAttribute(value = "The maximum amount of times a connection is used before it gets closed") + public int getMaxUsageCount() { - lock(); - try - { - return activeConnections.contains(connection); - } - finally - { - unlock(); - } + return super.getMaxUsageCount(); } @Override - protected void onCreated(Connection connection) + public void setMaxUsageCount(int maxUsageCount) { - lock(); - try - { - // Use "cold" new connections as last. - idleConnections.offer(connection); - } - finally - { - unlock(); - } - - idle(connection, false); - } - - @Override - protected Connection activate() - { - Connection connection; - lock(); - try - { - connection = idleConnections.poll(); - if (connection == null) - return null; - activeConnections.add(connection); - } - finally - { - unlock(); - } - - return active(connection); - } - - @Override - public boolean release(Connection connection) - { - boolean closed = isClosed(); - lock(); - try - { - if (!activeConnections.remove(connection)) - return false; - - if (!closed) - { - // Make sure we use "hot" connections first. - deactivate(connection); - } - } - finally - { - unlock(); - } - - released(connection); - return idle(connection, closed); - } - - protected boolean deactivate(Connection connection) - { - return idleConnections.offerFirst(connection); - } - - @Override - public boolean remove(Connection connection) - { - return remove(connection, false); - } - - protected boolean remove(Connection connection, boolean force) - { - boolean activeRemoved; - boolean idleRemoved; - lock(); - try - { - activeRemoved = activeConnections.remove(connection); - idleRemoved = idleConnections.remove(connection); - } - finally - { - unlock(); - } - - if (activeRemoved || force) - released(connection); - boolean removed = activeRemoved || idleRemoved || force; - if (removed) - removed(connection); - return removed; - } - - @Override - public void close() - { - super.close(); - - List connections = new ArrayList<>(); - lock(); - try - { - connections.addAll(idleConnections); - idleConnections.clear(); - connections.addAll(activeConnections); - activeConnections.clear(); - } - finally - { - unlock(); - } - - close(connections); - } - - @Override - public void dump(Appendable out, String indent) throws IOException - { - DumpableCollection active; - DumpableCollection idle; - lock(); - try - { - active = new DumpableCollection("active", new ArrayList<>(activeConnections)); - idle = new DumpableCollection("idle", new ArrayList<>(idleConnections)); - } - finally - { - unlock(); - } - dump(out, indent, active, idle); - } - - protected void dump(Appendable out, String indent, Object... items) throws IOException - { - Dumpable.dumpObjects(out, indent, this, items); - } - - @Override - public boolean sweep() - { - List toSweep; - lock(); - try - { - toSweep = activeConnections.stream() - .filter(connection -> connection instanceof Sweeper.Sweepable) - .collect(Collectors.toList()); - } - finally - { - unlock(); - } - - for (Connection connection : toSweep) - { - if (((Sweeper.Sweepable)connection).sweep()) - { - boolean removed = remove(connection, true); - LOG.warn("Connection swept: {}{}{} from active connections{}{}", - connection, - System.lineSeparator(), - removed ? "Removed" : "Not removed", - System.lineSeparator(), - dump()); - } - } - - return false; - } - - @Override - public String toString() - { - int activeSize; - int idleSize; - lock(); - try - { - activeSize = activeConnections.size(); - idleSize = idleConnections.size(); - } - finally - { - unlock(); - } - - return String.format("%s@%x[c=%d/%d/%d,a=%d,i=%d]", - getClass().getSimpleName(), - hashCode(), - getPendingConnectionCount(), - getConnectionCount(), - getMaxConnectionCount(), - activeSize, - idleSize); + super.setMaxUsageCount(maxUsageCount); } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java index 9abc17bce8e..b70044f983a 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java @@ -35,17 +35,19 @@ import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.util.Attachable; import org.eclipse.jetty.util.HttpCookieStore; import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public abstract class HttpConnection implements IConnection +public abstract class HttpConnection implements IConnection, Attachable { private static final Logger LOG = LoggerFactory.getLogger(HttpConnection.class); private final AutoLock lock = new AutoLock(); private final HttpDestination destination; + private Object attachment; private int idleTimeoutGuard; private long idleTimeoutStamp; @@ -273,6 +275,18 @@ public abstract class HttpConnection implements IConnection } } + @Override + public void setAttachment(Object obj) + { + this.attachment = obj; + } + + @Override + public Object getAttachment() + { + return attachment; + } + @Override public String toString() { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java index a93ed908a2c..175f397075d 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java @@ -427,6 +427,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest { if (connectionPool.isActive(connection)) { + // trigger the next request after releasing the connection if (connectionPool.release(connection)) send(false); else diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java index 82b1918428d..b2863fad1c2 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpProxy.java @@ -37,6 +37,7 @@ import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.io.ClientConnectionFactory; import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.util.Attachable; import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.slf4j.Logger; @@ -276,11 +277,12 @@ public class HttpProxy extends ProxyConfiguration.Proxy } } - private static class ProxyConnection implements Connection + private static class ProxyConnection implements Connection, Attachable { private final Destination destination; private final Connection connection; private final Promise promise; + private Object attachment; private ProxyConnection(Destination destination, Connection connection, Promise promise) { @@ -313,6 +315,18 @@ public class HttpProxy extends ProxyConfiguration.Proxy { return connection.isClosed(); } + + @Override + public void setAttachment(Object obj) + { + this.attachment = obj; + } + + @Override + public Object getAttachment() + { + return attachment; + } } private static class TunnelPromise implements Promise diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/LeakTrackingConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/LeakTrackingConnectionPool.java index 3fcec061c34..4c1c3ef314b 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/LeakTrackingConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/LeakTrackingConnectionPool.java @@ -39,7 +39,7 @@ public class LeakTrackingConnectionPool extends DuplexConnectionPool public LeakTrackingConnectionPool(HttpDestination destination, int maxConnections, Callback requester) { - super(destination, maxConnections, requester); + super((HttpDestination)destination, maxConnections, requester); start(); } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/MultiplexConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/MultiplexConnectionPool.java index 364a63dcfe1..09b1cb01f83 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/MultiplexConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/MultiplexConnectionPool.java @@ -18,309 +18,47 @@ package org.eclipse.jetty.client; -import java.io.IOException; -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.Deque; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; - -import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.util.Callback; -import org.eclipse.jetty.util.component.Dumpable; -import org.eclipse.jetty.util.component.DumpableCollection; -import org.eclipse.jetty.util.thread.AutoLock; -import org.eclipse.jetty.util.thread.Sweeper; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.annotation.ManagedObject; -public class MultiplexConnectionPool extends AbstractConnectionPool implements ConnectionPool.Multiplexable, Sweeper.Sweepable +@ManagedObject +public class MultiplexConnectionPool extends AbstractConnectionPool implements ConnectionPool.Multiplexable { - private static final Logger LOG = LoggerFactory.getLogger(MultiplexConnectionPool.class); - - private final AutoLock lock = new AutoLock(); - private final Deque idleConnections; - private final Map activeConnections; - private int maxMultiplex; - public MultiplexConnectionPool(HttpDestination destination, int maxConnections, Callback requester, int maxMultiplex) { - super(destination, maxConnections, requester); - this.idleConnections = new ArrayDeque<>(maxConnections); - this.activeConnections = new LinkedHashMap<>(maxConnections); - this.maxMultiplex = maxMultiplex; - } - - @Override - public Connection acquire(boolean create) - { - Connection connection = activate(); - if (connection == null && create) - { - int queuedRequests = getHttpDestination().getQueuedRequestCount(); - int maxMultiplex = getMaxMultiplex(); - int maxPending = ceilDiv(queuedRequests, maxMultiplex); - tryCreate(maxPending); - connection = activate(); - } - return connection; - } - - /** - * @param a the dividend - * @param b the divisor - * @return the ceiling of the algebraic quotient - */ - private static int ceilDiv(int a, int b) - { - return (a + b - 1) / b; + this(destination, maxConnections, true, requester, maxMultiplex); + } + + public MultiplexConnectionPool(HttpDestination destination, int maxConnections, boolean cache, Callback requester, int maxMultiplex) + { + super(destination, maxConnections, cache, requester); + setMaxMultiplex(maxMultiplex); } @Override + @ManagedAttribute(value = "The multiplexing factor of connections") public int getMaxMultiplex() { - try (AutoLock l = lock.lock()) - { - return maxMultiplex; - } + return super.getMaxMultiplex(); } @Override public void setMaxMultiplex(int maxMultiplex) { - try (AutoLock l = lock.lock()) - { - this.maxMultiplex = maxMultiplex; - } + super.setMaxMultiplex(maxMultiplex); } @Override - public boolean accept(Connection connection) + @ManagedAttribute(value = "The maximum amount of times a connection is used before it gets closed") + public int getMaxUsageCount() { - boolean accepted = super.accept(connection); - if (LOG.isDebugEnabled()) - LOG.debug("Accepted {} {}", accepted, connection); - if (accepted) - { - try (AutoLock l = lock.lock()) - { - Holder holder = new Holder(connection); - activeConnections.put(connection, holder); - ++holder.count; - } - active(connection); - } - return accepted; + return super.getMaxUsageCount(); } @Override - public boolean isActive(Connection connection) + public void setMaxUsageCount(int maxUsageCount) { - try (AutoLock l = lock.lock()) - { - return activeConnections.containsKey(connection); - } - } - - @Override - protected void onCreated(Connection connection) - { - try (AutoLock l = lock.lock()) - { - // Use "cold" connections as last. - idleConnections.offer(new Holder(connection)); - } - idle(connection, false); - } - - @Override - protected Connection activate() - { - Holder result = null; - try (AutoLock l = lock.lock()) - { - for (Holder holder : activeConnections.values()) - { - if (holder.count < maxMultiplex) - { - result = holder; - break; - } - } - - if (result == null) - { - Holder holder = idleConnections.poll(); - if (holder == null) - return null; - activeConnections.put(holder.connection, holder); - result = holder; - } - - ++result.count; - } - return active(result.connection); - } - - @Override - public boolean release(Connection connection) - { - boolean closed = isClosed(); - boolean idle = false; - Holder holder; - try (AutoLock l = lock.lock()) - { - holder = activeConnections.get(connection); - if (holder != null) - { - int count = --holder.count; - if (count == 0) - { - activeConnections.remove(connection); - if (!closed) - { - idleConnections.offerFirst(holder); - idle = true; - } - } - } - } - if (holder == null) - return false; - - released(connection); - if (idle || closed) - return idle(connection, closed); - return true; - } - - @Override - public boolean remove(Connection connection) - { - return remove(connection, false); - } - - protected boolean remove(Connection connection, boolean force) - { - boolean activeRemoved = true; - boolean idleRemoved = false; - try (AutoLock l = lock.lock()) - { - Holder holder = activeConnections.remove(connection); - if (holder == null) - { - activeRemoved = false; - for (Iterator iterator = idleConnections.iterator(); iterator.hasNext(); ) - { - holder = iterator.next(); - if (holder.connection == connection) - { - idleRemoved = true; - iterator.remove(); - break; - } - } - } - } - if (activeRemoved || force) - released(connection); - boolean removed = activeRemoved || idleRemoved || force; - if (removed) - removed(connection); - return removed; - } - - @Override - public void close() - { - super.close(); - List connections; - try (AutoLock l = lock.lock()) - { - connections = idleConnections.stream().map(holder -> holder.connection).collect(Collectors.toList()); - connections.addAll(activeConnections.keySet()); - } - close(connections); - } - - @Override - public void dump(Appendable out, String indent) throws IOException - { - DumpableCollection active; - DumpableCollection idle; - try (AutoLock l = lock.lock()) - { - active = new DumpableCollection("active", new ArrayList<>(activeConnections.values())); - idle = new DumpableCollection("idle", new ArrayList<>(idleConnections)); - } - Dumpable.dumpObjects(out, indent, this, active, idle); - } - - @Override - public boolean sweep() - { - List toSweep = new ArrayList<>(); - try (AutoLock l = lock.lock()) - { - activeConnections.values().stream() - .map(holder -> holder.connection) - .filter(connection -> connection instanceof Sweeper.Sweepable) - .collect(Collectors.toCollection(() -> toSweep)); - } - for (Connection connection : toSweep) - { - if (((Sweeper.Sweepable)connection).sweep()) - { - boolean removed = remove(connection, true); - LOG.warn("Connection swept: {}{}{} from active connections{}{}", - connection, - System.lineSeparator(), - removed ? "Removed" : "Not removed", - System.lineSeparator(), - dump()); - } - } - return false; - } - - @Override - public String toString() - { - int activeSize; - int idleSize; - try (AutoLock l = lock.lock()) - { - activeSize = activeConnections.size(); - idleSize = idleConnections.size(); - } - return String.format("%s@%x[connections=%d/%d/%d,multiplex=%d,active=%d,idle=%d]", - getClass().getSimpleName(), - hashCode(), - getPendingConnectionCount(), - getConnectionCount(), - getMaxConnectionCount(), - getMaxMultiplex(), - activeSize, - idleSize); - } - - private static class Holder - { - private final Connection connection; - private int count; - - private Holder(Connection connection) - { - this.connection = connection; - } - - @Override - public String toString() - { - return String.format("%s[%d]", connection, count); - } + super.setMaxUsageCount(maxUsageCount); } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/RoundRobinConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/RoundRobinConnectionPool.java index dbf8bde4699..825598b484e 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/RoundRobinConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/RoundRobinConnectionPool.java @@ -18,23 +18,22 @@ package org.eclipse.jetty.client; -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.Pool; import org.eclipse.jetty.util.annotation.ManagedObject; -import org.eclipse.jetty.util.component.Dumpable; -import org.eclipse.jetty.util.thread.AutoLock; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; @ManagedObject -public class RoundRobinConnectionPool extends AbstractConnectionPool implements ConnectionPool.Multiplexable +public class RoundRobinConnectionPool extends MultiplexConnectionPool { - private final AutoLock lock = new AutoLock(); - private final List entries; - private int maxMultiplex; - private int index; + private static final Logger LOG = LoggerFactory.getLogger(RoundRobinConnectionPool.class); + + private final AtomicInteger offset = new AtomicInteger(); + private final Pool pool; public RoundRobinConnectionPool(HttpDestination destination, int maxConnections, Callback requester) { @@ -43,220 +42,31 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements public RoundRobinConnectionPool(HttpDestination destination, int maxConnections, Callback requester, int maxMultiplex) { - super(destination, maxConnections, requester); - entries = new ArrayList<>(maxConnections); - for (int i = 0; i < maxConnections; ++i) - { - entries.add(new Entry()); - } - this.maxMultiplex = maxMultiplex; - } - - @Override - public int getMaxMultiplex() - { - try (AutoLock l = lock.lock()) - { - return maxMultiplex; - } - } - - @Override - public void setMaxMultiplex(int maxMultiplex) - { - try (AutoLock l = lock.lock()) - { - this.maxMultiplex = maxMultiplex; - } - } - - /** - *

Returns an idle connection, if available, following a round robin algorithm; - * otherwise it always tries to create a new connection, up until the max connection count.

- * - * @param create this parameter is ignored and assumed to be always {@code true} - * @return an idle connection or {@code null} if no idle connections are available - */ - @Override - public Connection acquire(boolean create) - { - // The nature of this connection pool is such that a - // connection must always be present in the next slot. - return super.acquire(true); - } - - @Override - protected void onCreated(Connection connection) - { - try (AutoLock l = lock.lock()) - { - for (Entry entry : entries) - { - if (entry.connection == null) - { - entry.connection = connection; - break; - } - } - } - idle(connection, false); + super(destination, maxConnections, false, requester, maxMultiplex); + pool = destination.getBean(Pool.class); } @Override protected Connection activate() { - Connection connection = null; - try (AutoLock l = lock.lock()) - { - int offset = 0; - int capacity = getMaxConnectionCount(); - while (offset < capacity) - { - int idx = index + offset; - if (idx >= capacity) - idx -= capacity; - - Entry entry = entries.get(idx); - - if (entry.connection == null) - break; - - if (entry.active < getMaxMultiplex()) - { - ++entry.active; - ++entry.used; - connection = entry.connection; - index += offset + 1; - if (index >= capacity) - index -= capacity; - break; - } - - ++offset; - } - } - return connection == null ? null : active(connection); + int offset = this.offset.get(); + Connection connection = activate(offset); + if (connection != null) + this.offset.getAndIncrement(); + return connection; } - @Override - public boolean isActive(Connection connection) + private Connection activate(int offset) { - try (AutoLock l = lock.lock()) + Pool.Entry entry = pool.acquireAt(Math.abs(offset % pool.getMaxEntries())); + if (LOG.isDebugEnabled()) + LOG.debug("activated '{}'", entry); + if (entry != null) { - for (Entry entry : entries) - { - if (entry.connection == connection) - return entry.active > 0; - } - return false; - } - } - - @Override - public boolean release(Connection connection) - { - boolean found = false; - boolean idle = false; - try (AutoLock l = lock.lock()) - { - for (Entry entry : entries) - { - if (entry.connection == connection) - { - found = true; - int active = --entry.active; - idle = active == 0; - break; - } - } - } - if (!found) - return false; - released(connection); - if (idle) - return idle(connection, isClosed()); - return true; - } - - @Override - public boolean remove(Connection connection) - { - boolean found = false; - try (AutoLock l = lock.lock()) - { - for (Entry entry : entries) - { - if (entry.connection == connection) - { - found = true; - entry.reset(); - break; - } - } - } - if (found) - { - released(connection); - removed(connection); - } - return found; - } - - @Override - public void dump(Appendable out, String indent) throws IOException - { - List connections; - try (AutoLock l = lock.lock()) - { - connections = new ArrayList<>(entries); - } - Dumpable.dumpObjects(out, indent, out, connections); - } - - @Override - public String toString() - { - int present = 0; - int active = 0; - try (AutoLock l = lock.lock()) - { - for (Entry entry : entries) - { - if (entry.connection != null) - { - ++present; - if (entry.active > 0) - ++active; - } - } - } - return String.format("%s@%x[c=%d/%d/%d,a=%d]", - getClass().getSimpleName(), - hashCode(), - getPendingConnectionCount(), - present, - getMaxConnectionCount(), - active - ); - } - - private static class Entry - { - private Connection connection; - private int active; - private long used; - - private void reset() - { - connection = null; - active = 0; - used = 0; - } - - @Override - public String toString() - { - return String.format("{u=%d,c=%s}", used, connection); + Connection connection = entry.getPooled(); + acquired(connection); + return connection; } + return null; } } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/ValidatingConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/ValidatingConnectionPool.java index 8007131c1b9..4ea8de50030 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/ValidatingConnectionPool.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/ValidatingConnectionPool.java @@ -19,15 +19,15 @@ package org.eclipse.jetty.client; import java.io.IOException; -import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Stream; import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.annotation.ManagedAttribute; +import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.DumpableCollection; import org.eclipse.jetty.util.thread.Scheduler; import org.slf4j.Logger; @@ -66,10 +66,10 @@ public class ValidatingConnectionPool extends DuplexConnectionPool public ValidatingConnectionPool(HttpDestination destination, int maxConnections, Callback requester, Scheduler scheduler, long timeout) { - super(destination, maxConnections, requester); + super((HttpDestination)destination, maxConnections, requester); this.scheduler = scheduler; this.timeout = timeout; - this.quarantine = new HashMap<>(maxConnections); + this.quarantine = new ConcurrentHashMap<>(maxConnections); } @ManagedAttribute(value = "The number of validating connections", readonly = true) @@ -81,21 +81,11 @@ public class ValidatingConnectionPool extends DuplexConnectionPool @Override public boolean release(Connection connection) { - lock(); - try - { - if (!getActiveConnections().remove(connection)) - return false; - Holder holder = new Holder(connection); - holder.task = scheduler.schedule(holder, timeout, TimeUnit.MILLISECONDS); - quarantine.put(connection, holder); - if (LOG.isDebugEnabled()) - LOG.debug("Validating for {}ms {}", timeout, connection); - } - finally - { - unlock(); - } + Holder holder = new Holder(connection); + holder.task = scheduler.schedule(holder, timeout, TimeUnit.MILLISECONDS); + quarantine.put(connection, holder); + if (LOG.isDebugEnabled()) + LOG.debug("Validating for {}ms {}", timeout, connection); released(connection); return true; @@ -104,16 +94,7 @@ public class ValidatingConnectionPool extends DuplexConnectionPool @Override public boolean remove(Connection connection) { - Holder holder; - lock(); - try - { - holder = quarantine.remove(connection); - } - finally - { - unlock(); - } + Holder holder = quarantine.remove(connection); if (holder == null) return super.remove(connection); @@ -129,25 +110,16 @@ public class ValidatingConnectionPool extends DuplexConnectionPool } @Override - protected void dump(Appendable out, String indent, Object... items) throws IOException + public void dump(Appendable out, String indent) throws IOException { DumpableCollection toDump = new DumpableCollection("quarantine", quarantine.values()); - super.dump(out, indent, Stream.concat(Stream.of(items), Stream.of(toDump))); + Dumpable.dumpObjects(out, indent, this, toDump); } @Override public String toString() { - int size; - lock(); - try - { - size = quarantine.size(); - } - finally - { - unlock(); - } + int size = quarantine.size(); return String.format("%s[v=%d]", super.toString(), size); } @@ -169,20 +141,11 @@ public class ValidatingConnectionPool extends DuplexConnectionPool if (done.compareAndSet(false, true)) { boolean closed = isClosed(); - lock(); - try - { - if (LOG.isDebugEnabled()) - LOG.debug("Validated {}", connection); - quarantine.remove(connection); - if (!closed) - deactivate(connection); - } - finally - { - unlock(); - } - + if (LOG.isDebugEnabled()) + LOG.debug("Validated {}", connection); + quarantine.remove(connection); + if (!closed) + deactivate(connection); idle(connection, closed); proceed(); } diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java index 48f82012363..b2e0a38aab8 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java @@ -44,12 +44,13 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.EndPoint; +import org.eclipse.jetty.util.Attachable; import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.thread.Sweeper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class HttpConnectionOverHTTP extends AbstractConnection implements IConnection, org.eclipse.jetty.io.Connection.UpgradeFrom, Sweeper.Sweepable +public class HttpConnectionOverHTTP extends AbstractConnection implements IConnection, org.eclipse.jetty.io.Connection.UpgradeFrom, Sweeper.Sweepable, Attachable { private static final Logger LOG = LoggerFactory.getLogger(HttpConnectionOverHTTP.class); @@ -161,6 +162,18 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements IConne return closed.get(); } + @Override + public void setAttachment(Object obj) + { + delegate.setAttachment(obj); + } + + @Override + public Object getAttachment() + { + return delegate.getAttachment(); + } + @Override public boolean onIdleExpired() { diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/ConnectionPoolHelper.java b/jetty-client/src/test/java/org/eclipse/jetty/client/ConnectionPoolHelper.java new file mode 100644 index 00000000000..2c04dc5dcd2 --- /dev/null +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/ConnectionPoolHelper.java @@ -0,0 +1,34 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.client; + +import org.eclipse.jetty.client.api.Connection; + +public class ConnectionPoolHelper +{ + public static Connection acquire(AbstractConnectionPool connectionPool, boolean create) + { + return connectionPool.acquire(create); + } + + public static void tryCreate(AbstractConnectionPool connectionPool, int pending) + { + connectionPool.tryCreate(pending); + } +} diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/DuplexHttpDestinationTest.java similarity index 77% rename from jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java rename to jetty-client/src/test/java/org/eclipse/jetty/client/DuplexHttpDestinationTest.java index f6aadd4e7a1..8b0c3df6b24 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/DuplexHttpDestinationTest.java @@ -16,7 +16,7 @@ // ======================================================================== // -package org.eclipse.jetty.client.http; +package org.eclipse.jetty.client; import java.lang.reflect.Method; import java.util.concurrent.CountDownLatch; @@ -24,21 +24,12 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; -import org.eclipse.jetty.client.AbstractHttpClientServerTest; -import org.eclipse.jetty.client.ConnectionPool; -import org.eclipse.jetty.client.DuplexConnectionPool; -import org.eclipse.jetty.client.DuplexHttpDestination; -import org.eclipse.jetty.client.EmptyServerHandler; -import org.eclipse.jetty.client.HttpClient; -import org.eclipse.jetty.client.HttpDestination; -import org.eclipse.jetty.client.Origin; import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.client.api.Destination; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; -import org.eclipse.jetty.util.Callback; import org.hamcrest.Matchers; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; @@ -52,7 +43,7 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest +public class DuplexHttpDestinationTest extends AbstractHttpClientServerTest { @ParameterizedTest @ArgumentsSource(ScenarioProvider.class) @@ -67,7 +58,7 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest Connection connection = connectionPool.acquire(true); assertNull(connection); // There are no queued requests, so no connection should be created. - connection = pollIdleConnection(connectionPool, 1, TimeUnit.SECONDS); + connection = peekIdleConnection(connectionPool, 1, TimeUnit.SECONDS); assertNull(connection); } } @@ -78,19 +69,19 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest { start(scenario, new EmptyServerHandler()); - try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort()))) + try (HttpDestination destination = new DuplexHttpDestination(client, new Origin("http", "localhost", connector.getLocalPort()))) { destination.start(); - TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool(); + DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); // Trigger creation of one connection. - connectionPool.tryCreate(1); + ConnectionPoolHelper.tryCreate(connectionPool, 1); - Connection connection = connectionPool.acquire(false); + Connection connection = ConnectionPoolHelper.acquire(connectionPool, false); if (connection == null) { // There are no queued requests, so the newly created connection will be idle - connection = pollIdleConnection(connectionPool, 5, TimeUnit.SECONDS); + connection = peekIdleConnection(connectionPool, 5, TimeUnit.SECONDS); } assertNotNull(connection); } @@ -102,13 +93,13 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest { start(scenario, new EmptyServerHandler()); - try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort()))) + try (HttpDestination destination = new DuplexHttpDestination(client, new Origin("http", "localhost", connector.getLocalPort()))) { destination.start(); - TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool(); + DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); // Trigger creation of one connection. - connectionPool.tryCreate(1); + ConnectionPoolHelper.tryCreate(connectionPool, 1); Connection connection1 = connectionPool.acquire(true); if (connection1 == null) @@ -131,12 +122,12 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest CountDownLatch idleLatch = new CountDownLatch(1); CountDownLatch latch = new CountDownLatch(1); - try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort())) + try (HttpDestination destination = new DuplexHttpDestination(client, new Origin("http", "localhost", connector.getLocalPort())) { @Override protected ConnectionPool newConnectionPool(HttpClient client) { - return new TestConnectionPool(this, client.getMaxConnectionsPerDestination(), this) + return new DuplexConnectionPool(this, client.getMaxConnectionsPerDestination(), this) { @Override protected void onCreated(Connection connection) @@ -157,10 +148,10 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest }) { destination.start(); - TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool(); + DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); // Trigger creation of one connection. - connectionPool.tryCreate(1); + ConnectionPoolHelper.tryCreate(connectionPool, 1); // Make sure we entered idleCreated(). assertTrue(idleLatch.await(5, TimeUnit.SECONDS)); @@ -171,7 +162,7 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest assertNull(connection1); // Trigger creation of a second connection. - connectionPool.tryCreate(1); + ConnectionPoolHelper.tryCreate(connectionPool, 1); // Second attempt also returns null because we delayed idleCreated() above. Connection connection2 = connectionPool.acquire(true); @@ -180,9 +171,9 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest latch.countDown(); // There must be 2 idle connections. - Connection connection = pollIdleConnection(connectionPool, 5, TimeUnit.SECONDS); + Connection connection = peekIdleConnection(connectionPool, 5, TimeUnit.SECONDS); assertNotNull(connection); - connection = pollIdleConnection(connectionPool, 5, TimeUnit.SECONDS); + connection = peekIdleConnection(connectionPool, 5, TimeUnit.SECONDS); assertNotNull(connection); } } @@ -193,13 +184,13 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest { start(scenario, new EmptyServerHandler()); - try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort()))) + try (HttpDestination destination = new DuplexHttpDestination(client, new Origin("http", "localhost", connector.getLocalPort()))) { destination.start(); - TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool(); + DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); // Trigger creation of one connection. - connectionPool.tryCreate(1); + ConnectionPoolHelper.tryCreate(connectionPool, 1); Connection connection1 = connectionPool.acquire(true); if (connection1 == null) @@ -230,13 +221,13 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest long idleTimeout = 1000; startClient(scenario, httpClient -> httpClient.setIdleTimeout(idleTimeout)); - try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort()))) + try (HttpDestination destination = new DuplexHttpDestination(client, new Origin("http", "localhost", connector.getLocalPort()))) { destination.start(); - TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool(); + DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); // Trigger creation of one connection. - connectionPool.tryCreate(1); + ConnectionPoolHelper.tryCreate(connectionPool, 1); Connection connection1 = connectionPool.acquire(true); if (connection1 == null) @@ -247,7 +238,7 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest TimeUnit.MILLISECONDS.sleep(2 * idleTimeout); - connection1 = connectionPool.getIdleConnections().poll(); + connection1 = connectionPool.getIdleConnections().peek(); assertNull(connection1); } } @@ -353,11 +344,6 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest assertTrue(client.getDestinations().isEmpty(), "Destination must be removed after connection error"); } - private Connection pollIdleConnection(DuplexConnectionPool connectionPool, long time, TimeUnit unit) throws InterruptedException - { - return await(() -> connectionPool.getIdleConnections().poll(), time, unit); - } - private Connection peekIdleConnection(DuplexConnectionPool connectionPool, long time, TimeUnit unit) throws InterruptedException { return await(() -> connectionPool.getIdleConnections().peek(), time, unit); @@ -375,38 +361,4 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest } return null; } - - private static class TestDestination extends DuplexHttpDestination - { - public TestDestination(HttpClient client, Origin origin) - { - super(client, origin); - } - - @Override - protected ConnectionPool newConnectionPool(HttpClient client) - { - return new TestConnectionPool(this, client.getMaxConnectionsPerDestination(), this); - } - - public static class TestConnectionPool extends DuplexConnectionPool - { - public TestConnectionPool(HttpDestination destination, int maxConnections, Callback requester) - { - super(destination, maxConnections, requester); - } - - @Override - public void tryCreate(int maxPending) - { - super.tryCreate(maxPending); - } - - @Override - public Connection acquire(boolean create) - { - return super.acquire(create); - } - } - } } diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java index 491df8bdbea..1f066bb1bbe 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java @@ -661,7 +661,7 @@ public class HttpClientTLSTest HttpDestination destination = client.resolveDestination(origin); DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); // Trigger the creation of a new connection, but don't use it. - connectionPool.tryCreate(-1); + ConnectionPoolHelper.tryCreate(connectionPool, -1); // Verify that the connection has been created. while (true) { @@ -759,7 +759,7 @@ public class HttpClientTLSTest HttpDestination destination = client.resolveDestination(origin); DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); // Trigger the creation of a new connection, but don't use it. - connectionPool.tryCreate(-1); + ConnectionPoolHelper.tryCreate(connectionPool, -1); // Verify that the connection has been created. while (true) { diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java index a06fd83957d..f430b9db61f 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpConnectionLifecycleTest.java @@ -74,17 +74,14 @@ public class HttpConnectionLifecycleTest extends AbstractHttpClientServerTest HttpDestination destination = (HttpDestination)client.resolveDestination(request); DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); - Collection idleConnections = connectionPool.getIdleConnections(); - assertEquals(0, idleConnections.size()); - - Collection activeConnections = connectionPool.getActiveConnections(); - assertEquals(0, activeConnections.size()); + assertEquals(0, connectionPool.getIdleConnections().size()); + assertEquals(0, connectionPool.getActiveConnections().size()); request.onRequestSuccess(r -> successLatch.countDown()) .onResponseHeaders(response -> { - assertEquals(0, idleConnections.size()); - assertEquals(1, activeConnections.size()); + assertEquals(0, ((DuplexConnectionPool)destination.getConnectionPool()).getIdleConnections().size()); + assertEquals(1, ((DuplexConnectionPool)destination.getConnectionPool()).getActiveConnections().size()); headersLatch.countDown(); }) .send(new Response.Listener.Adapter() @@ -106,8 +103,8 @@ public class HttpConnectionLifecycleTest extends AbstractHttpClientServerTest assertTrue(headersLatch.await(30, TimeUnit.SECONDS)); assertTrue(successLatch.await(30, TimeUnit.SECONDS)); - assertEquals(1, idleConnections.size()); - assertEquals(0, activeConnections.size()); + assertEquals(1, ((DuplexConnectionPool)destination.getConnectionPool()).getIdleConnections().size()); + assertEquals(0, ((DuplexConnectionPool)destination.getConnectionPool()).getActiveConnections().size()); } @ParameterizedTest @@ -124,18 +121,15 @@ public class HttpConnectionLifecycleTest extends AbstractHttpClientServerTest HttpDestination destination = (HttpDestination)client.resolveDestination(request); DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool(); - Collection idleConnections = connectionPool.getIdleConnections(); - assertEquals(0, idleConnections.size()); - - Collection activeConnections = connectionPool.getActiveConnections(); - assertEquals(0, activeConnections.size()); + assertEquals(0, connectionPool.getIdleConnections().size()); + assertEquals(0, connectionPool.getActiveConnections().size()); request.listener(new Request.Listener.Adapter() { @Override public void onBegin(Request request) { - activeConnections.iterator().next().close(); + connectionPool.getActiveConnections().iterator().next().close(); beginLatch.countDown(); } @@ -144,24 +138,23 @@ public class HttpConnectionLifecycleTest extends AbstractHttpClientServerTest { failureLatch.countDown(); } - }) - .send(new Response.Listener.Adapter() + }).send(new Response.Listener.Adapter() + { + @Override + public void onComplete(Result result) { - @Override - public void onComplete(Result result) - { - assertTrue(result.isFailed()); - assertEquals(0, idleConnections.size()); - assertEquals(0, activeConnections.size()); - failureLatch.countDown(); - } - }); + assertTrue(result.isFailed()); + assertEquals(0, connectionPool.getIdleConnections().size()); + assertEquals(0, connectionPool.getActiveConnections().size()); + failureLatch.countDown(); + } + }); assertTrue(beginLatch.await(30, TimeUnit.SECONDS)); assertTrue(failureLatch.await(30, TimeUnit.SECONDS)); - assertEquals(0, idleConnections.size()); - assertEquals(0, activeConnections.size()); + assertEquals(0, connectionPool.getIdleConnections().size()); + assertEquals(0, connectionPool.getActiveConnections().size()); } @ParameterizedTest diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java index 01f971a32e8..6dfbe2728ea 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java @@ -51,6 +51,7 @@ import org.eclipse.jetty.io.AbstractConnection; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.RetainableByteBuffer; +import org.eclipse.jetty.util.Attachable; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Promise; @@ -58,7 +59,7 @@ import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class HttpConnectionOverFCGI extends AbstractConnection implements IConnection +public class HttpConnectionOverFCGI extends AbstractConnection implements IConnection, Attachable { private static final Logger LOG = LoggerFactory.getLogger(HttpConnectionOverFCGI.class); @@ -73,6 +74,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne private final Delegate delegate; private final ClientParser parser; private RetainableByteBuffer networkBuffer; + private Object attachment; public HttpConnectionOverFCGI(EndPoint endPoint, HttpDestination destination, Promise promise) { @@ -267,6 +269,18 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne return closed.get(); } + @Override + public void setAttachment(Object obj) + { + this.attachment = obj; + } + + @Override + public Object getAttachment() + { + return attachment; + } + protected boolean closeByHTTP(HttpFields fields) { if (!fields.contains(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString())) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/IStream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/IStream.java index a2f58724ca0..3747150c354 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/IStream.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/IStream.java @@ -22,6 +22,7 @@ import java.io.Closeable; import org.eclipse.jetty.http2.api.Stream; import org.eclipse.jetty.http2.frames.Frame; +import org.eclipse.jetty.util.Attachable; import org.eclipse.jetty.util.Callback; /** @@ -29,21 +30,8 @@ import org.eclipse.jetty.util.Callback; *

This class extends {@link Stream} by adding the methods required to * implement the HTTP/2 stream functionalities.

*/ -public interface IStream extends Stream, Closeable +public interface IStream extends Stream, Attachable, Closeable { - /** - * @return the object attached to this stream - * @see #setAttachment(Object) - */ - public Object getAttachment(); - - /** - * Attaches the given object to this stream for later retrieval. - * - * @param attachment the object to attach to this stream - */ - public void setAttachment(Object attachment); - /** * @return whether this stream is local or remote */ diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java index 1edef46d8f3..a2a26fb2887 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java @@ -72,24 +72,6 @@ public class OpenIdCredentials implements Serializable return response; } - public boolean isExpired() - { - if (authCode != null || claims == null) - return true; - - // Check expiry - long expiry = (Long)claims.get("exp"); - long currentTimeSeconds = (long)(System.currentTimeMillis() / 1000F); - if (currentTimeSeconds > expiry) - { - if (LOG.isDebugEnabled()) - LOG.debug("OpenId Credentials expired {}", this); - return true; - } - - return false; - } - public void redeemAuthCode(OpenIdConfiguration configuration) throws Exception { if (LOG.isDebugEnabled()) @@ -105,15 +87,15 @@ public class OpenIdCredentials implements Serializable String idToken = (String)response.get("id_token"); if (idToken == null) - throw new IllegalArgumentException("no id_token"); + throw new AuthenticationException("no id_token"); String accessToken = (String)response.get("access_token"); if (accessToken == null) - throw new IllegalArgumentException("no access_token"); + throw new AuthenticationException("no access_token"); String tokenType = (String)response.get("token_type"); if (!"Bearer".equalsIgnoreCase(tokenType)) - throw new IllegalArgumentException("invalid token_type"); + throw new AuthenticationException("invalid token_type"); claims = JwtDecoder.decode(idToken); if (LOG.isDebugEnabled()) @@ -128,11 +110,11 @@ public class OpenIdCredentials implements Serializable } } - private void validateClaims(OpenIdConfiguration configuration) + private void validateClaims(OpenIdConfiguration configuration) throws Exception { // Issuer Identifier for the OpenID Provider MUST exactly match the value of the iss (issuer) Claim. if (!configuration.getIssuer().equals(claims.get("iss"))) - throw new IllegalArgumentException("Issuer Identifier MUST exactly match the iss Claim"); + throw new AuthenticationException("Issuer Identifier MUST exactly match the iss Claim"); // The aud (audience) Claim MUST contain the client_id value. validateAudience(configuration); @@ -140,10 +122,16 @@ public class OpenIdCredentials implements Serializable // If an azp (authorized party) Claim is present, verify that its client_id is the Claim Value. Object azp = claims.get("azp"); if (azp != null && !configuration.getClientId().equals(azp)) - throw new IllegalArgumentException("Authorized party claim value should be the client_id"); + throw new AuthenticationException("Authorized party claim value should be the client_id"); + + // Check that the ID token has not expired by checking the exp claim. + long expiry = (Long)claims.get("exp"); + long currentTimeSeconds = (long)(System.currentTimeMillis() / 1000F); + if (currentTimeSeconds > expiry) + throw new AuthenticationException("ID Token has expired"); } - private void validateAudience(OpenIdConfiguration configuration) + private void validateAudience(OpenIdConfiguration configuration) throws AuthenticationException { Object aud = claims.get("aud"); String clientId = configuration.getClientId(); @@ -152,17 +140,17 @@ public class OpenIdCredentials implements Serializable boolean isValidType = isString || isList; if (isString && !clientId.equals(aud)) - throw new IllegalArgumentException("Audience Claim MUST contain the client_id value"); + throw new AuthenticationException("Audience Claim MUST contain the client_id value"); else if (isList) { if (!Arrays.asList((Object[])aud).contains(clientId)) - throw new IllegalArgumentException("Audience Claim MUST contain the client_id value"); + throw new AuthenticationException("Audience Claim MUST contain the client_id value"); if (claims.get("azp") == null) - throw new IllegalArgumentException("A multi-audience ID token needs to contain an azp claim"); + throw new AuthenticationException("A multi-audience ID token needs to contain an azp claim"); } else if (!isValidType) - throw new IllegalArgumentException("Audience claim was not valid"); + throw new AuthenticationException("Audience claim was not valid"); } @SuppressWarnings("unchecked") @@ -185,7 +173,15 @@ public class OpenIdCredentials implements Serializable Object parsedResponse = new JSON().fromJSON(responseBody); if (!(parsedResponse instanceof Map)) - throw new IllegalStateException("Malformed response from OpenID Provider"); + throw new AuthenticationException("Malformed response from OpenID Provider"); return (Map)parsedResponse; } + + public static class AuthenticationException extends Exception + { + public AuthenticationException(String message) + { + super(message); + } + } } diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java index e67cdf6a493..fa0db1d2cb7 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java @@ -18,7 +18,6 @@ package org.eclipse.jetty.security.openid; -import java.security.Principal; import javax.security.auth.Subject; import javax.servlet.ServletRequest; @@ -86,8 +85,6 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi try { openIdCredentials.redeemAuthCode(configuration); - if (openIdCredentials.isExpired()) - return null; } catch (Throwable e) { @@ -138,12 +135,10 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi @Override public boolean validate(UserIdentity user) { - Principal userPrincipal = user.getUserPrincipal(); - if (!(userPrincipal instanceof OpenIdUserPrincipal)) + if (!(user.getUserPrincipal() instanceof OpenIdUserPrincipal)) return false; - OpenIdCredentials credentials = ((OpenIdUserPrincipal)userPrincipal).getCredentials(); - return !credentials.isExpired(); + return loginService == null || loginService.validate(user); } @Override @@ -167,5 +162,7 @@ public class OpenIdLoginService extends ContainerLifeCycle implements LoginServi @Override public void logout(UserIdentity user) { + if (loginService != null) + loginService.logout(user); } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java index 81cd58d8b00..df7850d9b1e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java @@ -410,7 +410,7 @@ public class Dispatcher implements RequestDispatcher case INCLUDE_CONTEXT_PATH: { ContextHandler.Context context = _baseRequest.getContext(); - return context == null ? null : context.getContextHandler().getContextPathEncoded(); + return context == null ? null : context.getContextHandler().getRequestContextPath(); } case INCLUDE_QUERY_STRING: return _query; 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 e1c00421666..16da17fd3d9 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 @@ -794,14 +794,7 @@ public class Request implements HttpServletRequest if (context == null) return null; - // For some reason the spec requires the context path to be encoded (unlike getServletPath). - String contextPath = context.getContextHandler().getContextPathEncoded(); - - // For the root context, the spec requires that the empty string is returned instead of the leading '/' - // which is included in the pathInContext - if (URIUtil.SLASH.equals(contextPath)) - return ""; - return contextPath; + return context.getContextHandler().getRequestContextPath(); } /** Get the path in the context. @@ -1714,7 +1707,7 @@ public class Request implements HttpServletRequest // TODO this is not really right for CONNECT path = _uri.isAbsolute() ? "/" : null; else if (encoded.startsWith("/")) - path = (encoded.length() == 1) ? "/" : _uri.getDecodedPath(); + path = (encoded.length() == 1) ? "/" : URIUtil.canonicalPath(_uri.getDecodedPath()); else if ("*".equals(encoded) || HttpMethod.CONNECT.is(getMethod())) path = encoded; else diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 72b77cd68af..5e993ffd648 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -41,6 +41,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.DispatcherType; import javax.servlet.Filter; import javax.servlet.FilterRegistration; @@ -225,11 +226,14 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu public enum Availability { - UNAVAILABLE, STARTING, AVAILABLE, SHUTDOWN, + STOPPED, // stopped and can't be made unavailable nor shutdown + STARTING, // starting inside of doStart. It may go to any of the next states. + AVAILABLE, // running normally + UNAVAILABLE, // Either a startup error or explicit call to setAvailable(false) + SHUTDOWN, // graceful shutdown } - ; - private volatile Availability _availability = Availability.UNAVAILABLE; + private final AtomicReference _availability = new AtomicReference<>(Availability.STOPPED); public ContextHandler() { @@ -727,7 +731,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @ManagedAttribute("true for graceful shutdown, which allows existing requests to complete") public boolean isShutdown() { - return _availability == Availability.SHUTDOWN; + return _availability.get() == Availability.SHUTDOWN; } /** @@ -737,10 +741,25 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @Override public CompletableFuture shutdown() { - _availability = isRunning() ? Availability.SHUTDOWN : Availability.UNAVAILABLE; - CompletableFuture shutdown = new CompletableFuture(); - shutdown.complete(null); - return shutdown; + while (true) + { + Availability availability = _availability.get(); + switch (availability) + { + case STOPPED: + return CompletableFuture.failedFuture(new IllegalStateException(getState())); + case STARTING: + case AVAILABLE: + case UNAVAILABLE: + if (!_availability.compareAndSet(availability, Availability.SHUTDOWN)) + continue; + break; + default: + break; + } + break; + } + return CompletableFuture.completedFuture(null); } /** @@ -748,7 +767,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public boolean isAvailable() { - return _availability == Availability.AVAILABLE; + return _availability.get() == Availability.AVAILABLE; } /** @@ -758,12 +777,46 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void setAvailable(boolean available) { - try (AutoLock l = _lock.lock()) + // Only supported state transitions are: + // UNAVAILABLE --true---> AVAILABLE + // STARTING -----false--> UNAVAILABLE + // AVAILABLE ----false--> UNAVAILABLE + if (available) { - if (available && isRunning()) - _availability = Availability.AVAILABLE; - else if (!available || !isRunning()) - _availability = Availability.UNAVAILABLE; + while (true) + { + Availability availability = _availability.get(); + switch (availability) + { + case AVAILABLE: + break; + case UNAVAILABLE: + if (!_availability.compareAndSet(availability, Availability.AVAILABLE)) + continue; + break; + default: + throw new IllegalStateException(availability.toString()); + } + break; + } + } + else + { + while (true) + { + Availability availability = _availability.get(); + switch (availability) + { + case STARTING: + case AVAILABLE: + if (!_availability.compareAndSet(availability, Availability.UNAVAILABLE)) + continue; + break; + default: + break; + } + break; + } } } @@ -780,7 +833,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @Override protected void doStart() throws Exception { - _availability = Availability.STARTING; + _availability.set(Availability.STARTING); if (_contextPath == null) throw new IllegalStateException("Null contextPath"); @@ -817,13 +870,12 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu contextInitialized(); - _availability = Availability.AVAILABLE; + _availability.compareAndSet(Availability.STARTING, Availability.AVAILABLE); LOG.info("Started {}", this); } finally { - if (_availability == Availability.STARTING) - _availability = Availability.UNAVAILABLE; + _availability.compareAndSet(Availability.STARTING, Availability.UNAVAILABLE); exitScope(null); __context.set(oldContext); // reset the classloader @@ -971,7 +1023,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu { if (getServer().isDryRun()) return; - + if (LOG.isDebugEnabled()) LOG.debug("contextInitialized: {}->{}", e, l); l.contextInitialized(e); @@ -981,7 +1033,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu { if (getServer().isDryRun()) return; - + if (LOG.isDebugEnabled()) LOG.debug("contextDestroyed: {}->{}", e, l); l.contextDestroyed(e); @@ -993,7 +1045,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu // Should we attempt a graceful shutdown? MultiException mex = null; - _availability = Availability.UNAVAILABLE; + _availability.set(Availability.STOPPED); ClassLoader oldClassloader = null; ClassLoader oldWebapploader = null; @@ -1148,8 +1200,10 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu return false; } - switch (_availability) + switch (_availability.get()) { + case STOPPED: + return false; case SHUTDOWN: case UNAVAILABLE: baseRequest.setHandled(true); @@ -1504,6 +1558,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void setClassLoader(ClassLoader classLoader) { + if (isStarted()) + throw new IllegalStateException(getState()); _classLoader = classLoader; } @@ -1776,7 +1832,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu b.append('{'); if (getDisplayName() != null) b.append(getDisplayName()).append(','); - b.append(getContextPath()).append(',').append(getBaseResource()).append(',').append(_availability); + b.append(getContextPath()).append(',').append(getBaseResource()).append(',').append(_availability.get()); if (vhosts != null && vhosts.length > 0) b.append(',').append(vhosts[0]); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/LargeHeaderTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/LargeHeaderTest.java index c303cd94a3e..c7de36cff1f 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/LargeHeaderTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/LargeHeaderTest.java @@ -37,8 +37,6 @@ import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.LifeCycle; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -100,7 +98,6 @@ public class LargeHeaderTest @Test public void testLargeHeader() throws Throwable { - final Logger CLIENTLOG = Log.getLogger(LargeHeaderTest.class).getLogger(".client"); ExecutorService executorService = Executors.newFixedThreadPool(8); int localPort = server.getURI().getPort(); @@ -127,7 +124,6 @@ public class LargeHeaderTest } catch (Throwable t) { - CLIENTLOG.warn("Client Issue", t); issues.addSuppressed(t); } }); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java index e4f3557d8eb..9e970c49764 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java @@ -417,6 +417,18 @@ public class ContextHandlerTest assertThat(connector.getResponse("GET /foo/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo'")); assertThat(connector.getResponse("GET /foo/bar/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo/bar'")); + // If we make foobar unavailable, then requests will be handled by 503 + foobar.setAvailable(false); + assertThat(connector.getResponse("GET / HTTP/1.0\n\n"), Matchers.containsString("ctx=''")); + assertThat(connector.getResponse("GET /foo/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo'")); + assertThat(connector.getResponse("GET /foo/bar/xxx HTTP/1.0\n\n"), Matchers.containsString(" 503 ")); + + // If we make foobar available, then requests will be handled normally + foobar.setAvailable(true); + assertThat(connector.getResponse("GET / HTTP/1.0\n\n"), Matchers.containsString("ctx=''")); + assertThat(connector.getResponse("GET /foo/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo'")); + assertThat(connector.getResponse("GET /foo/bar/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo/bar'")); + // If we stop foobar, then requests will be handled by foo foobar.stop(); assertThat(connector.getResponse("GET / HTTP/1.0\n\n"), Matchers.containsString("ctx=''")); diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index ed3b4218f0b..4b14ecd539a 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -450,8 +450,6 @@ public class ServletHandler extends ScopedHandler // Get the base requests final ServletPathMapping old_servlet_path_mapping = baseRequest.getServletPathMapping(); - DispatcherType type = baseRequest.getDispatcherType(); - ServletHolder servletHolder = null; UserIdentity.Scope oldScope = null; @@ -483,8 +481,7 @@ public class ServletHandler extends ScopedHandler if (oldScope != null) baseRequest.setUserIdentityScope(oldScope); - if (!(DispatcherType.INCLUDE.equals(type))) - baseRequest.setServletPathMapping(old_servlet_path_mapping); + baseRequest.setServletPathMapping(old_servlet_path_mapping); } } diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletRangesTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletRangesTest.java index d1412064f9f..c3a3cf0d173 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletRangesTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletRangesTest.java @@ -150,12 +150,28 @@ public class DefaultServletRangesTest String boundary = body.substring(0, body.indexOf("\r\n")); assertResponseContains("206 Partial", response); assertResponseContains("Content-Type: multipart/byteranges; boundary=", response); - assertResponseContains("Content-Range: bytes 0-9/80", response); - assertResponseContains("Content-Range: bytes 20-29/80", response); - assertResponseContains("Content-Range: bytes 40-49/80", response); - assertResponseContains(DATA.substring(0, 10), response); - assertResponseContains(DATA.substring(20, 30), response); - assertResponseContains(DATA.substring(40, 50), response); + + String section1 = boundary + "\r\n" + + "Content-Type: text/plain\r\n" + + "Content-Range: bytes 0-9/80\r\n" + + "\r\n" + + DATA.substring(0, 10) + "\r\n"; + assertResponseContains(section1, response); + + String section2 = boundary + "\r\n" + + "Content-Type: text/plain\r\n" + + "Content-Range: bytes 20-29/80\r\n" + + "\r\n" + + DATA.substring(20, 30) + "\r\n"; + assertResponseContains(section2, response); + + String section3 = boundary + "\r\n" + + "Content-Type: text/plain\r\n" + + "Content-Range: bytes 40-49/80\r\n" + + "\r\n" + + DATA.substring(40, 50) + "\r\n"; + assertResponseContains(section3, response); + assertTrue(body.endsWith(boundary + "--\r\n")); } diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java index a6dcb6e8a9f..816cde4232f 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DispatcherTest.java @@ -57,6 +57,7 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.ResourceHandler; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.UrlEncoded; import org.junit.jupiter.api.AfterEach; @@ -834,14 +835,14 @@ public class DispatcherTest assertEquals(null, request.getPathInfo()); assertEquals(null, request.getPathTranslated()); - UrlEncoded query = new UrlEncoded(); - query.decode(request.getQueryString()); + MultiMap query = new MultiMap<>(); + UrlEncoded.decodeTo(request.getQueryString(), query, UrlEncoded.ENCODING); assertThat(query.getString("do"), is("end")); // Russian for "selected=Temperature" - UrlEncoded q2 = new UrlEncoded(); - q2.decode(query.getString("else")); - String russian = q2.encode(); + MultiMap q2 = new MultiMap<>(); + UrlEncoded.decodeTo(query.getString("else"), q2, UrlEncoded.ENCODING); + String russian = UrlEncoded.encode(q2, UrlEncoded.ENCODING, false); assertThat(russian, is("%D0%B2%D1%8B%D0%B1%D1%80%D0%B0%D0%BD%D0%BE=%D0%A2%D0%B5%D0%BC%D0%BF%D0%B5%D1%80%D0%B0%D1%82%D1%83%D1%80%D0%B0")); assertThat(query.containsKey("test"), is(false)); assertThat(query.containsKey("foreign"), is(false)); diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/IncludedServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/IncludedServletTest.java index c7794d7e99d..c3413e93ba8 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/IncludedServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/IncludedServletTest.java @@ -42,7 +42,6 @@ import org.eclipse.jetty.toolchain.test.IO; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -108,12 +107,13 @@ public class IncludedServletTest private void dumpAttrs(String tag, HttpServletRequest req, ServletOutputStream out) throws IOException { - out.println(String.format("%s: %s='%s'", tag, RequestDispatcher.INCLUDE_CONTEXT_PATH, - req.getAttribute(RequestDispatcher.INCLUDE_CONTEXT_PATH))); - out.println(String.format("%s: %s='%s'", tag, RequestDispatcher.INCLUDE_SERVLET_PATH, - req.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH))); - out.println(String.format("%s: %s='%s'", tag, RequestDispatcher.INCLUDE_PATH_INFO, - req.getAttribute(RequestDispatcher.INCLUDE_PATH_INFO))); + String contextPath = (String)req.getAttribute(RequestDispatcher.INCLUDE_CONTEXT_PATH); + String servletPath = (String)req.getAttribute(RequestDispatcher.INCLUDE_SERVLET_PATH); + String pathInfo = (String)req.getAttribute(RequestDispatcher.INCLUDE_PATH_INFO); + + out.println(String.format("%s: %s='%s'", tag, RequestDispatcher.INCLUDE_CONTEXT_PATH, contextPath)); + out.println(String.format("%s: %s='%s'", tag, RequestDispatcher.INCLUDE_SERVLET_PATH, servletPath)); + out.println(String.format("%s: %s='%s'", tag, RequestDispatcher.INCLUDE_PATH_INFO, pathInfo)); } } @@ -221,7 +221,6 @@ public class IncludedServletTest } } - @Disabled // TODO: complete merge of PR #5058. @Test public void testIncludeAttributes() throws IOException { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Attachable.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Attachable.java new file mode 100644 index 00000000000..f5007cb53bd --- /dev/null +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Attachable.java @@ -0,0 +1,38 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.util; + +/** + * Abstract mechanism to support attachment of miscellaneous objects. + */ +public interface Attachable +{ + /** + * @return the object attached to this stream + * @see #setAttachment(Object) + */ + Object getAttachment(); + + /** + * Attaches the given object to this stream for later retrieval. + * + * @param attachment the object to attach to this stream + */ + void setAttachment(Object attachment); +} diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java index 67c748d0d92..c0cce0430e7 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java @@ -29,8 +29,8 @@ import java.nio.charset.StandardCharsets; public class MultiPartOutputStream extends FilterOutputStream { - private static final byte[] __CRLF = {'\r', '\n'}; - private static final byte[] __DASHDASH = {'-', '-'}; + private static final byte[] CRLF = {'\r', '\n'}; + private static final byte[] DASHDASH = {'-', '-'}; public static final String MULTIPART_MIXED = "multipart/mixed"; public static final String MULTIPART_X_MIXED_REPLACE = "multipart/x-mixed-replace"; @@ -71,11 +71,11 @@ public class MultiPartOutputStream extends FilterOutputStream try { if (inPart) - out.write(__CRLF); - out.write(__DASHDASH); + out.write(CRLF); + out.write(DASHDASH); out.write(boundaryBytes); - out.write(__DASHDASH); - out.write(__CRLF); + out.write(DASHDASH); + out.write(CRLF); inPart = false; } finally @@ -104,15 +104,19 @@ public class MultiPartOutputStream extends FilterOutputStream throws IOException { if (inPart) - out.write(__CRLF); + { + out.write(CRLF); + } inPart = true; - out.write(__DASHDASH); + out.write(DASHDASH); out.write(boundaryBytes); - out.write(__CRLF); + out.write(CRLF); if (contentType != null) + { out.write(("Content-Type: " + contentType).getBytes(StandardCharsets.ISO_8859_1)); - out.write(__CRLF); - out.write(__CRLF); + out.write(CRLF); + } + out.write(CRLF); } /** @@ -126,20 +130,22 @@ public class MultiPartOutputStream extends FilterOutputStream throws IOException { if (inPart) - out.write(__CRLF); + out.write(CRLF); inPart = true; - out.write(__DASHDASH); + out.write(DASHDASH); out.write(boundaryBytes); - out.write(__CRLF); + out.write(CRLF); if (contentType != null) + { out.write(("Content-Type: " + contentType).getBytes(StandardCharsets.ISO_8859_1)); - out.write(__CRLF); + out.write(CRLF); + } for (int i = 0; headers != null && i < headers.length; i++) { out.write(headers[i].getBytes(StandardCharsets.ISO_8859_1)); - out.write(__CRLF); + out.write(CRLF); } - out.write(__CRLF); + out.write(CRLF); } @Override @@ -148,7 +154,3 @@ public class MultiPartOutputStream extends FilterOutputStream out.write(b, off, len); } } - - - - diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java new file mode 100644 index 00000000000..e3de1bb28af --- /dev/null +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Pool.java @@ -0,0 +1,446 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.util; + +import java.io.Closeable; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.CopyOnWriteArrayList; + +import org.eclipse.jetty.util.component.Dumpable; +import org.eclipse.jetty.util.thread.AutoLock; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A fast container of poolable objects, with optional support for + * multiplexing, max usage count and thread-local caching. + *

+ * The thread-local caching mechanism is about remembering up to N previously + * used entries into a thread-local single-threaded collection. + * When that collection is not empty, its entries are removed one by one + * during acquisition until an entry that can be acquired is found. + * This can greatly speed up acquisition when both the acquisition and the + * release of the entries is done on the same thread as this avoids iterating + * the global, thread-safe collection of entries. + * @param + */ +public class Pool implements AutoCloseable, Dumpable +{ + private static final Logger LOGGER = LoggerFactory.getLogger(Pool.class); + + private final List sharedList = new CopyOnWriteArrayList<>(); + /* + * The cache is used to avoid hammering on the first index of the entry list. + * Caches can become poisoned (i.e.: containing entries that are in use) when + * the release isn't done by the acquiring thread or when the entry pool is + * undersized compared to the load applied on it. + * When an entry can't be found in the cache, the global list is iterated + * normally so the cache has no visible effect besides performance. + */ + private final ThreadLocal> cache; + private final AutoLock lock = new AutoLock(); + private final int maxEntries; + private final int cacheSize; + private volatile boolean closed; + private volatile int maxMultiplex = 1; + private volatile int maxUsageCount = -1; + + /** + * Construct a Pool with the specified thread-local cache size. + * + * @param maxEntries the maximum amount of entries that the pool will accept. + * @param cacheSize the thread-local cache size. A value less than 1 means the cache is disabled. + */ + public Pool(int maxEntries, int cacheSize) + { + this.maxEntries = maxEntries; + this.cacheSize = cacheSize; + if (cacheSize > 0) + this.cache = ThreadLocal.withInitial(() -> new ArrayList(cacheSize)); + else + this.cache = null; + } + + public int getPendingConnectionCount() + { + return (int)sharedList.stream().filter(entry -> entry.getPooled() == null).count(); + } + + public int getIdleConnectionCount() + { + return (int)sharedList.stream().filter(Entry::isIdle).count(); + } + + public int getInUseConnectionCount() + { + return (int)sharedList.stream().filter(entry -> !entry.isIdle()).count(); + } + + public int getMaxEntries() + { + return maxEntries; + } + + public int getMaxMultiplex() + { + return maxMultiplex; + } + + public final void setMaxMultiplex(int maxMultiplex) + { + if (maxMultiplex < 1) + throw new IllegalArgumentException("Max multiplex must be >= 1"); + this.maxMultiplex = maxMultiplex; + } + + public int getMaxUsageCount() + { + return maxUsageCount; + } + + public final void setMaxUsageCount(int maxUsageCount) + { + if (maxUsageCount == 0) + throw new IllegalArgumentException("Max usage count must be != 0"); + this.maxUsageCount = maxUsageCount; + } + + /** + * Create a new disabled slot into the pool. The returned entry + * won't be acquirable as long as {@link Entry#enable(Object)} + * has not been called. + * + * @param maxReservations the max desired number of reserved entries, + * or a negative number to always trigger the reservation of a new entry. + * @return a disabled entry that is contained in the pool, + * or null if the pool is closed or if the pool already contains + * {@link #getMaxEntries()} entries. + */ + public Entry reserve(int maxReservations) + { + if (maxReservations >= 0 && getPendingConnectionCount() >= maxReservations) + return null; + + try (AutoLock l = lock.lock()) + { + if (!closed && sharedList.size() < maxEntries) + { + Entry entry = new Entry(); + sharedList.add(entry); + return entry; + } + return null; + } + } + + /** + * Acquire the entry from the pool at the specified index. This method bypasses the thread-local mechanism. + * + * @param idx the index of the entry to acquire. + * @return the specified entry or null if there is none at the specified index or if it is not available. + */ + public Entry acquireAt(int idx) + { + if (closed) + return null; + + try + { + Entry entry = sharedList.get(idx); + if (entry.tryAcquire()) + return entry; + } + catch (IndexOutOfBoundsException e) + { + // no entry at that index + } + return null; + } + + /** + * Acquire an entry from the pool. + * + * @return an entry from the pool or null if none is available. + */ + public Entry acquire() + { + if (closed) + return null; + + // first check the thread-local cache + if (cache != null) + { + List cachedList = cache.get(); + while (!cachedList.isEmpty()) + { + Entry cachedEntry = cachedList.remove(cachedList.size() - 1); + if (cachedEntry.tryAcquire()) + return cachedEntry; + } + } + + // then iterate the shared list + for (Entry entry : sharedList) + { + if (entry.tryAcquire()) + return entry; + } + return null; + } + + /** + * This method will return an acquired object to the pool. Objects + * that are acquired from the pool but never released will result + * in a memory leak. + * + * @param entry the value to return to the pool + * @return true if the entry was released and could be acquired again, + * false if the entry should be removed by calling {@link #remove(Pool.Entry)} + * and the object contained by the entry should be disposed. + * @throws NullPointerException if value is null + */ + public boolean release(Entry entry) + { + if (closed) + return false; + + // first mark it as unused + boolean reusable = entry.tryRelease(); + + // then cache the released entry + if (cache != null && reusable) + { + List cachedList = cache.get(); + if (cachedList.size() < cacheSize) + cachedList.add(entry); + } + return reusable; + } + + /** + * Remove a value from the pool. + * + * @param entry the value to remove + * @return true if the entry was removed, false otherwise + */ + public boolean remove(Entry entry) + { + if (closed) + return false; + + if (!entry.tryRemove()) + { + if (LOGGER.isDebugEnabled()) + LOGGER.debug("Attempt to remove an object from the pool that is still in use: {}", entry); + return false; + } + + boolean removed = sharedList.remove(entry); + if (!removed) + { + if (LOGGER.isDebugEnabled()) + LOGGER.debug("Attempt to remove an object from the pool that does not exist: {}", entry); + } + + return removed; + } + + public boolean isClosed() + { + return closed; + } + + @Override + public void close() + { + List copy; + try (AutoLock l = lock.lock()) + { + closed = true; + copy = new ArrayList<>(sharedList); + sharedList.clear(); + } + + // iterate the copy and close its entries + for (Entry entry : copy) + { + if (entry.tryRemove() && entry.pooled instanceof Closeable) + { + try + { + ((Closeable)entry.pooled).close(); + } + catch (IOException e) + { + LOGGER.warn("Error closing entry {}", entry, e); + } + } + } + } + + public int size() + { + return sharedList.size(); + } + + public Collection values() + { + return Collections.unmodifiableCollection(sharedList); + } + + @Override + public void dump(Appendable out, String indent) throws IOException + { + Dumpable.dumpObjects(out, indent, this); + } + + @Override + public String toString() + { + return getClass().getSimpleName() + " size=" + sharedList.size() + " closed=" + closed + " entries=" + sharedList; + } + + public class Entry + { + // hi: positive=open/maxUsage counter,negative=closed lo: multiplexing counter + private final AtomicBiInteger state; + private volatile T pooled; + + public Entry() + { + this.state = new AtomicBiInteger(-1, 0); + } + + public T getPooled() + { + return pooled; + } + + public void enable(T pooled) + { + if (!isClosed()) + throw new IllegalStateException("Open entries cannot be enabled : " + this); + Objects.requireNonNull(pooled); + this.pooled = pooled; + state.set(0, 0); + } + + /** + * Try to acquire the entry if possible by incrementing both the usage + * count and the multiplex count. + * @return true if the usage count is <= maxUsageCount and + * the multiplex count is maxMultiplex and the entry is not closed, + * false otherwise. + */ + public boolean tryAcquire() + { + while (true) + { + long encoded = state.get(); + int usageCount = AtomicBiInteger.getHi(encoded); + boolean closed = usageCount < 0; + int multiplexingCount = AtomicBiInteger.getLo(encoded); + int currentMaxUsageCount = maxUsageCount; + if (closed || multiplexingCount >= maxMultiplex || (currentMaxUsageCount > 0 && usageCount >= currentMaxUsageCount)) + return false; + + if (state.compareAndSet(encoded, usageCount + 1, multiplexingCount + 1)) + return true; + } + } + + /** + * Try to release the entry if possible by decrementing the multiplexing + * count unless the entity is closed. + * @return true if the entry was released, + * false if {@link #tryRemove()} should be called. + */ + public boolean tryRelease() + { + int newMultiplexingCount; + int usageCount; + while (true) + { + long encoded = state.get(); + usageCount = AtomicBiInteger.getHi(encoded); + boolean closed = usageCount < 0; + if (closed) + return false; + + newMultiplexingCount = AtomicBiInteger.getLo(encoded) - 1; + if (newMultiplexingCount < 0) + throw new IllegalStateException("Cannot release an already released entry"); + + if (state.compareAndSet(encoded, usageCount, newMultiplexingCount)) + break; + } + + int currentMaxUsageCount = maxUsageCount; + boolean overUsed = currentMaxUsageCount > 0 && usageCount >= currentMaxUsageCount; + return !(overUsed && newMultiplexingCount == 0); + } + + /** + * Try to mark the entry as removed. + * @return true if the entry has to be removed from the containing pool, false otherwise. + */ + public boolean tryRemove() + { + while (true) + { + long encoded = state.get(); + int usageCount = AtomicBiInteger.getHi(encoded); + int multiplexCount = AtomicBiInteger.getLo(encoded); + int newMultiplexCount = Math.max(multiplexCount - 1, 0); + + boolean removed = state.compareAndSet(usageCount, -1, multiplexCount, newMultiplexCount); + if (removed) + return newMultiplexCount == 0; + } + } + + public boolean isClosed() + { + return state.getHi() < 0; + } + + public boolean isIdle() + { + return state.getLo() <= 0; + } + + public int getUsageCount() + { + return Math.max(state.getHi(), 0); + } + + @Override + public String toString() + { + long encoded = state.get(); + return super.toString() + " stateHi=" + AtomicBiInteger.getHi(encoded) + + " stateLo=" + AtomicBiInteger.getLo(encoded) + " pooled=" + pooled; + } + } +} diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java b/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java index 32ff9592f67..82f7594c026 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java @@ -44,19 +44,11 @@ import static org.eclipse.jetty.util.TypeUtil.convertHexDigit; * passing a parameter or setting the "org.eclipse.jetty.util.UrlEncoding.charset" * System property. *

- *

- * The hashtable either contains String single values, vectors - * of String or arrays of Strings. - *

- *

- * This class is only partially synchronised. In particular, simple - * get operations are not protected from concurrent updates. - *

* * @see java.net.URLEncoder */ @SuppressWarnings("serial") -public class UrlEncoded extends MultiMap implements Cloneable +public class UrlEncoded { static final Logger LOG = LoggerFactory.getLogger(UrlEncoded.class); @@ -87,62 +79,8 @@ public class UrlEncoded extends MultiMap implements Cloneable ENCODING = encoding; } - public UrlEncoded(UrlEncoded url) + private UrlEncoded() { - super(url); - } - - public UrlEncoded() - { - } - - public UrlEncoded(String query) - { - decodeTo(query, this, ENCODING); - } - - public void decode(String query) - { - decodeTo(query, this, ENCODING); - } - - public void decode(String query, Charset charset) - { - decodeTo(query, this, charset); - } - - /** - * Encode MultiMap with % encoding for UTF8 sequences. - * - * @return the MultiMap as a string with % encoding - */ - public String encode() - { - return encode(ENCODING, false); - } - - /** - * Encode MultiMap with % encoding for arbitrary Charset sequences. - * - * @param charset the charset to use for encoding - * @return the MultiMap as a string encoded with % encodings - */ - public String encode(Charset charset) - { - return encode(charset, false); - } - - /** - * Encode MultiMap with % encoding. - * - * @param charset the charset to encode with - * @param equalsForNullValue if True, then an '=' is always used, even - * for parameters without a value. e.g. "blah?a=&b=&c=". - * @return the MultiMap as a string encoded with % encodings - */ - public synchronized String encode(Charset charset, boolean equalsForNullValue) - { - return encode(this, charset, equalsForNullValue); } /** @@ -190,11 +128,10 @@ public class UrlEncoded extends MultiMap implements Cloneable if (val != null) { - String str = val; - if (str.length() > 0) + if (val.length() > 0) { result.append('='); - result.append(encodeString(str, charset)); + result.append(encodeString(val, charset)); } else if (equalsForNullValue) result.append('='); @@ -228,6 +165,18 @@ public class UrlEncoded extends MultiMap implements Cloneable * @param charset the charset to use for decoding */ public static void decodeTo(String content, MultiMap map, Charset charset) + { + decodeTo(content, map, charset, -1); + } + + /** + * Decoded parameters to Map. + * + * @param content the string containing the encoded parameters + * @param map the MultiMap to put parsed query parameters into + * @param charset the charset to use for decoding + */ + public static void decodeTo(String content, MultiMap map, Charset charset, int maxKeys) { if (charset == null) charset = ENCODING; @@ -238,64 +187,62 @@ public class UrlEncoded extends MultiMap implements Cloneable return; } - synchronized (map) + String key = null; + String value; + int mark = -1; + boolean encoded = false; + for (int i = 0; i < content.length(); i++) { - String key = null; - String value; - int mark = -1; - boolean encoded = false; - for (int i = 0; i < content.length(); i++) + char c = content.charAt(i); + switch (c) { - char c = content.charAt(i); - switch (c) - { - case '&': - int l = i - mark - 1; - value = l == 0 ? "" : (encoded ? decodeString(content, mark + 1, l, charset) : content.substring(mark + 1, i)); - mark = i; - encoded = false; - if (key != null) - { - map.add(key, value); - } - else if (value != null && value.length() > 0) - { - map.add(value, ""); - } - key = null; - value = null; + case '&': + int l = i - mark - 1; + value = l == 0 ? "" : (encoded ? decodeString(content, mark + 1, l, charset) : content.substring(mark + 1, i)); + mark = i; + encoded = false; + if (key != null) + { + map.add(key, value); + } + else if (value != null && value.length() > 0) + { + map.add(value, ""); + } + checkMaxKeys(map, maxKeys); + key = null; + value = null; + break; + case '=': + if (key != null) break; - case '=': - if (key != null) - break; - key = encoded ? decodeString(content, mark + 1, i - mark - 1, charset) : content.substring(mark + 1, i); - mark = i; - encoded = false; - break; - case '+': - encoded = true; - break; - case '%': - encoded = true; - break; - } + key = encoded ? decodeString(content, mark + 1, i - mark - 1, charset) : content.substring(mark + 1, i); + mark = i; + encoded = false; + break; + case '+': + case '%': + encoded = true; + break; } + } - if (key != null) + if (key != null) + { + int l = content.length() - mark - 1; + value = l == 0 ? "" : (encoded ? decodeString(content, mark + 1, l, charset) : content.substring(mark + 1)); + map.add(key, value); + checkMaxKeys(map, maxKeys); + } + else if (mark < content.length()) + { + key = encoded + ? decodeString(content, mark + 1, content.length() - mark - 1, charset) + : content.substring(mark + 1); + if (key != null && key.length() > 0) { - int l = content.length() - mark - 1; - value = l == 0 ? "" : (encoded ? decodeString(content, mark + 1, l, charset) : content.substring(mark + 1)); - map.add(key, value); - } - else if (mark < content.length()) - { - key = encoded - ? decodeString(content, mark + 1, content.length() - mark - 1, charset) - : content.substring(mark + 1); - if (key != null && key.length() > 0) - { - map.add(key, ""); - } + map.add(key, ""); + checkMaxKeys(map, maxKeys); } } } @@ -316,76 +263,73 @@ public class UrlEncoded extends MultiMap implements Cloneable public static void decodeUtf8To(String query, int offset, int length, MultiMap map) { Utf8StringBuilder buffer = new Utf8StringBuilder(); - synchronized (map) + String key = null; + String value; + + int end = offset + length; + for (int i = offset; i < end; i++) { - String key = null; - String value = null; - - int end = offset + length; - for (int i = offset; i < end; i++) + char c = query.charAt(i); + switch (c) { - char c = query.charAt(i); - switch (c) - { - case '&': - value = buffer.toReplacedString(); - buffer.reset(); - if (key != null) - { - map.add(key, value); - } - else if (value != null && value.length() > 0) - { - map.add(value, ""); - } - key = null; - value = null; - break; + case '&': + value = buffer.toReplacedString(); + buffer.reset(); + if (key != null) + { + map.add(key, value); + } + else if (value != null && value.length() > 0) + { + map.add(value, ""); + } + key = null; + break; - case '=': - if (key != null) - { - buffer.append(c); - break; - } - key = buffer.toReplacedString(); - buffer.reset(); - break; - - case '+': - buffer.append((byte)' '); - break; - - case '%': - if (i + 2 < end) - { - char hi = query.charAt(++i); - char lo = query.charAt(++i); - buffer.append(decodeHexByte(hi, lo)); - } - else - { - throw new Utf8Appendable.NotUtf8Exception("Incomplete % encoding"); - } - break; - - default: + case '=': + if (key != null) + { buffer.append(c); break; - } - } + } + key = buffer.toReplacedString(); + buffer.reset(); + break; - if (key != null) - { - value = buffer.toReplacedString(); - buffer.reset(); - map.add(key, value); - } - else if (buffer.length() > 0) - { - map.add(buffer.toReplacedString(), ""); + case '+': + buffer.append((byte)' '); + break; + + case '%': + if (i + 2 < end) + { + char hi = query.charAt(++i); + char lo = query.charAt(++i); + buffer.append(decodeHexByte(hi, lo)); + } + else + { + throw new Utf8Appendable.NotUtf8Exception("Incomplete % encoding"); + } + break; + + default: + buffer.append(c); + break; } } + + if (key != null) + { + value = buffer.toReplacedString(); + buffer.reset(); + map.add(key, value); + } + else if (buffer.length() > 0) + { + map.add(buffer.toReplacedString(), ""); + } + } /** @@ -400,74 +344,70 @@ public class UrlEncoded extends MultiMap implements Cloneable public static void decode88591To(InputStream in, MultiMap map, int maxLength, int maxKeys) throws IOException { - synchronized (map) + StringBuilder buffer = new StringBuilder(); + String key = null; + String value; + + int b; + + int totalLength = 0; + while ((b = in.read()) >= 0) { - StringBuilder buffer = new StringBuilder(); - String key = null; - String value = null; - - int b; - - int totalLength = 0; - while ((b = in.read()) >= 0) + switch ((char)b) { - switch ((char)b) - { - case '&': - value = buffer.length() == 0 ? "" : buffer.toString(); - buffer.setLength(0); - if (key != null) - { - map.add(key, value); - } - else if (value.length() > 0) - { - map.add(value, ""); - } - key = null; - value = null; - checkMaxKeys(map, maxKeys); - break; + case '&': + value = buffer.length() == 0 ? "" : buffer.toString(); + buffer.setLength(0); + if (key != null) + { + map.add(key, value); + } + else if (value.length() > 0) + { + map.add(value, ""); + } + key = null; + checkMaxKeys(map, maxKeys); + break; - case '=': - if (key != null) - { - buffer.append((char)b); - break; - } - key = buffer.toString(); - buffer.setLength(0); - break; - - case '+': - buffer.append(' '); - break; - - case '%': - int code0 = in.read(); - int code1 = in.read(); - buffer.append(decodeHexChar(code0, code1)); - break; - - default: + case '=': + if (key != null) + { buffer.append((char)b); break; - } - checkMaxLength(++totalLength, maxLength); - } + } + key = buffer.toString(); + buffer.setLength(0); + break; - if (key != null) - { - value = buffer.length() == 0 ? "" : buffer.toString(); - buffer.setLength(0); - map.add(key, value); + case '+': + buffer.append(' '); + break; + + case '%': + int code0 = in.read(); + int code1 = in.read(); + buffer.append(decodeHexChar(code0, code1)); + break; + + default: + buffer.append((char)b); + break; } - else if (buffer.length() > 0) - { - map.add(buffer.toString(), ""); - } - checkMaxKeys(map, maxKeys); + checkMaxLength(++totalLength, maxLength); } + + if (key != null) + { + value = buffer.length() == 0 ? "" : buffer.toString(); + buffer.setLength(0); + map.add(key, value); + } + else if (buffer.length() > 0) + { + map.add(buffer.toString(), ""); + } + checkMaxKeys(map, maxKeys); } /** @@ -482,74 +422,70 @@ public class UrlEncoded extends MultiMap implements Cloneable public static void decodeUtf8To(InputStream in, MultiMap map, int maxLength, int maxKeys) throws IOException { - synchronized (map) + Utf8StringBuilder buffer = new Utf8StringBuilder(); + String key = null; + String value; + + int b; + + int totalLength = 0; + while ((b = in.read()) >= 0) { - Utf8StringBuilder buffer = new Utf8StringBuilder(); - String key = null; - String value = null; - - int b; - - int totalLength = 0; - while ((b = in.read()) >= 0) + switch ((char)b) { - switch ((char)b) - { - case '&': - value = buffer.toReplacedString(); - buffer.reset(); - if (key != null) - { - map.add(key, value); - } - else if (value != null && value.length() > 0) - { - map.add(value, ""); - } - key = null; - value = null; - checkMaxKeys(map, maxKeys); - break; + case '&': + value = buffer.toReplacedString(); + buffer.reset(); + if (key != null) + { + map.add(key, value); + } + else if (value != null && value.length() > 0) + { + map.add(value, ""); + } + key = null; + checkMaxKeys(map, maxKeys); + break; - case '=': - if (key != null) - { - buffer.append((byte)b); - break; - } - key = buffer.toReplacedString(); - buffer.reset(); - break; - - case '+': - buffer.append((byte)' '); - break; - - case '%': - char code0 = (char)in.read(); - char code1 = (char)in.read(); - buffer.append(decodeHexByte(code0, code1)); - break; - - default: + case '=': + if (key != null) + { buffer.append((byte)b); break; - } - checkMaxLength(++totalLength, maxLength); - } + } + key = buffer.toReplacedString(); + buffer.reset(); + break; - if (key != null) - { - value = buffer.toReplacedString(); - buffer.reset(); - map.add(key, value); + case '+': + buffer.append((byte)' '); + break; + + case '%': + char code0 = (char)in.read(); + char code1 = (char)in.read(); + buffer.append(decodeHexByte(code0, code1)); + break; + + default: + buffer.append((byte)b); + break; } - else if (buffer.length() > 0) - { - map.add(buffer.toReplacedString(), ""); - } - checkMaxKeys(map, maxKeys); + checkMaxLength(++totalLength, maxLength); } + + if (key != null) + { + value = buffer.toReplacedString(); + buffer.reset(); + map.add(key, value); + } + else if (buffer.length() > 0) + { + map.add(buffer.toReplacedString(), ""); + } + checkMaxKeys(map, maxKeys); } public static void decodeUtf16To(InputStream in, MultiMap map, int maxLength, int maxKeys) throws IOException @@ -558,8 +494,7 @@ public class UrlEncoded extends MultiMap implements Cloneable StringWriter buf = new StringWriter(8192); IO.copy(input, buf, maxLength); - // TODO implement maxKeys - decodeTo(buf.getBuffer().toString(), map, StandardCharsets.UTF_16); + decodeTo(buf.getBuffer().toString(), map, StandardCharsets.UTF_16, maxKeys); } /** @@ -627,77 +562,73 @@ public class UrlEncoded extends MultiMap implements Cloneable return; } - synchronized (map) + String key = null; + String value; + + int c; + + int totalLength = 0; + + try (ByteArrayOutputStream2 output = new ByteArrayOutputStream2()) { - String key = null; - String value = null; + int size; - int c; - - int totalLength = 0; - - try (ByteArrayOutputStream2 output = new ByteArrayOutputStream2()) + while ((c = in.read()) > 0) { - int size = 0; - - while ((c = in.read()) > 0) + switch ((char)c) { - switch ((char)c) - { - case '&': - size = output.size(); - value = size == 0 ? "" : output.toString(charset); - output.setCount(0); - if (key != null) - { - map.add(key, value); - } - else if (value != null && value.length() > 0) - { - map.add(value, ""); - } - key = null; - value = null; - checkMaxKeys(map, maxKeys); - break; - case '=': - if (key != null) - { - output.write(c); - break; - } - size = output.size(); - key = size == 0 ? "" : output.toString(charset); - output.setCount(0); - break; - case '+': - output.write(' '); - break; - case '%': - int code0 = in.read(); - int code1 = in.read(); - output.write(decodeHexChar(code0, code1)); - break; - default: + case '&': + size = output.size(); + value = size == 0 ? "" : output.toString(charset); + output.setCount(0); + if (key != null) + { + map.add(key, value); + } + else if (value != null && value.length() > 0) + { + map.add(value, ""); + } + key = null; + checkMaxKeys(map, maxKeys); + break; + case '=': + if (key != null) + { output.write(c); break; - } - checkMaxLength(++totalLength, maxLength); + } + size = output.size(); + key = size == 0 ? "" : output.toString(charset); + output.setCount(0); + break; + case '+': + output.write(' '); + break; + case '%': + int code0 = in.read(); + int code1 = in.read(); + output.write(decodeHexChar(code0, code1)); + break; + default: + output.write(c); + break; } - - size = output.size(); - if (key != null) - { - value = size == 0 ? "" : output.toString(charset); - output.setCount(0); - map.add(key, value); - } - else if (size > 0) - { - map.add(output.toString(charset), ""); - } - checkMaxKeys(map, maxKeys); + checkMaxLength(++totalLength, maxLength); } + + size = output.size(); + if (key != null) + { + value = size == 0 ? "" : output.toString(charset); + output.setCount(0); + map.add(key, value); + } + else if (size > 0) + { + map.add(output.toString(charset), ""); + } + checkMaxKeys(map, maxKeys); } } @@ -747,7 +678,7 @@ public class UrlEncoded extends MultiMap implements Cloneable for (int i = 0; i < length; i++) { char c = encoded.charAt(offset + i); - if (c < 0 || c > 0xff) + if (c > 0xff) { if (buffer == null) { @@ -808,7 +739,7 @@ public class UrlEncoded extends MultiMap implements Cloneable for (int i = 0; i < length; i++) { char c = encoded.charAt(offset + i); - if (c < 0 || c > 0xff) + if (c > 0xff) { if (buffer == null) { @@ -838,7 +769,7 @@ public class UrlEncoded extends MultiMap implements Cloneable byte[] ba = new byte[length]; int n = 0; - while (c >= 0 && c <= 0xff) + while (c <= 0xff) { if (c == '%') { @@ -935,18 +866,15 @@ public class UrlEncoded extends MultiMap implements Cloneable { if (charset == null) charset = ENCODING; - byte[] bytes = null; + byte[] bytes; bytes = string.getBytes(charset); - int len = bytes.length; byte[] encoded = new byte[bytes.length * 3]; int n = 0; boolean noEncode = true; - for (int i = 0; i < len; i++) + for (byte b : bytes) { - byte b = bytes[i]; - if (b == ' ') { noEncode = false; @@ -981,10 +909,4 @@ public class UrlEncoded extends MultiMap implements Cloneable return new String(encoded, 0, n, charset); } - - @Override - public Object clone() - { - return new UrlEncoded(this); - } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java index 164a702c99d..b5e54c60eab 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java @@ -139,4 +139,54 @@ public interface Graceful return CompletableFuture.allOf(gracefuls.stream().map(Graceful::shutdown).toArray(CompletableFuture[]::new)); } + + /** + * Utility method to execute a {@link ThrowingRunnable} in a new daemon thread and + * be notified of the result in a {@link CompletableFuture}. + * @param runnable the ThrowingRunnable to run. + * @return the CompletableFuture to be notified when the runnable either completes or fails. + */ + static CompletableFuture shutdown(ThrowingRunnable runnable) + { + AtomicReference stopThreadReference = new AtomicReference<>(); + CompletableFuture shutdown = new CompletableFuture<>() + { + @Override + public boolean cancel(boolean mayInterruptIfRunning) + { + boolean canceled = super.cancel(mayInterruptIfRunning); + if (canceled && mayInterruptIfRunning) + { + Thread thread = stopThreadReference.get(); + if (thread != null) + thread.interrupt(); + } + + return canceled; + } + }; + + Thread stopThread = new Thread(() -> + { + try + { + runnable.run(); + shutdown.complete(null); + } + catch (Throwable t) + { + shutdown.completeExceptionally(t); + } + }); + stopThread.setDaemon(true); + stopThreadReference.set(stopThread); + stopThread.start(); + return shutdown; + } + + @FunctionalInterface + interface ThrowingRunnable + { + void run() throws Exception; + } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java index 8cd13eff164..ee919a2a6b5 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/KeyStoreScanner.java @@ -27,8 +27,8 @@ import org.eclipse.jetty.util.Scanner; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedOperation; import org.eclipse.jetty.util.component.ContainerLifeCycle; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** *

The {@link KeyStoreScanner} is used to monitor the KeyStore file used by the {@link SslContextFactory}. @@ -38,7 +38,7 @@ import org.eclipse.jetty.util.log.Logger; */ public class KeyStoreScanner extends ContainerLifeCycle implements Scanner.DiscreteListener { - private static final Logger LOG = Log.getLogger(KeyStoreScanner.class); + private static final Logger LOG = LoggerFactory.getLogger(KeyStoreScanner.class); private final SslContextFactory sslContextFactory; private final File keystoreFile; diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/PoolTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/PoolTest.java new file mode 100644 index 00000000000..5b811728867 --- /dev/null +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/PoolTest.java @@ -0,0 +1,442 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.util; + +import java.io.Closeable; +import java.util.Arrays; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.jupiter.api.Test; + +import static java.util.stream.Collectors.toList; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class PoolTest +{ + + @Test + public void testAcquireRelease() + { + Pool pool = new Pool<>(1,0); + pool.reserve(-1).enable("aaa"); + + assertThat(pool.values().stream().findFirst().get().isIdle(), is(true)); + Pool.Entry e1 = pool.acquire(); + assertThat(e1.getPooled(), equalTo("aaa")); + assertThat(pool.values().stream().findFirst().get().isIdle(), is(false)); + assertThat(pool.acquire(), nullValue()); + assertThat(pool.release(e1), is(true)); + assertThat(pool.values().stream().findFirst().get().isIdle(), is(true)); + assertThrows(IllegalStateException.class, () -> pool.release(e1)); + Pool.Entry e2 = pool.acquire(); + assertThat(e2.getPooled(), equalTo("aaa")); + assertThat(pool.release(e2), is(true)); + assertThrows(NullPointerException.class, () -> pool.release(null)); + } + + @Test + public void testRemoveBeforeRelease() + { + Pool pool = new Pool<>(1,0); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e1 = pool.acquire(); + assertThat(pool.remove(e1), is(true)); + assertThat(pool.remove(e1), is(false)); + assertThat(pool.release(e1), is(false)); + } + + @Test + public void testCloseBeforeRelease() + { + Pool pool = new Pool<>(1,0); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e1 = pool.acquire(); + assertThat(pool.size(), is(1)); + pool.close(); + assertThat(pool.size(), is(0)); + assertThat(pool.release(e1), is(false)); + } + + @Test + public void testMaxPoolSize() + { + Pool pool = new Pool<>(1, 0); + assertThat(pool.size(), is(0)); + assertThat(pool.reserve(-1), notNullValue()); + assertThat(pool.size(), is(1)); + assertThat(pool.reserve(-1), nullValue()); + assertThat(pool.size(), is(1)); + } + + @Test + public void testReserve() + { + Pool pool = new Pool<>(2, 0); + Pool.Entry entry = pool.reserve(-1); + assertThat(pool.size(), is(1)); + assertThat(pool.acquire(), nullValue()); + assertThat(entry.isClosed(), is(true)); + + assertThrows(NullPointerException.class, () -> entry.enable(null)); + assertThat(pool.acquire(), nullValue()); + assertThat(entry.isClosed(), is(true)); + + entry.enable("aaa"); + assertThat(entry.isClosed(), is(false)); + assertThat(pool.acquire().getPooled(), notNullValue()); + + assertThrows(IllegalStateException.class, () -> entry.enable("bbb")); + + Pool.Entry e2 = pool.reserve(-1); + assertThat(pool.size(), is(2)); + assertThat(pool.remove(e2), is(true)); + assertThat(pool.size(), is(1)); + + pool.reserve(-1); + assertThat(pool.size(), is(2)); + pool.close(); + assertThat(pool.size(), is(0)); + assertThat(pool.reserve(-1), nullValue()); + assertThat(entry.isClosed(), is(true)); + } + + @Test + public void testReserveMaxPending() + { + Pool pool = new Pool<>(2, 0); + assertThat(pool.reserve(0), nullValue()); + assertThat(pool.reserve(1), notNullValue()); + assertThat(pool.reserve(1), nullValue()); + assertThat(pool.reserve(2), notNullValue()); + assertThat(pool.reserve(2), nullValue()); + assertThat(pool.reserve(3), nullValue()); + assertThat(pool.reserve(-1), nullValue()); + } + + @Test + public void testReserveNegativeMaxPending() + { + Pool pool = new Pool<>(2, 0); + assertThat(pool.reserve(-1), notNullValue()); + assertThat(pool.reserve(-1), notNullValue()); + assertThat(pool.reserve(-1), nullValue()); + } + + @Test + public void testClose() + { + Pool pool = new Pool<>(1, 0); + pool.reserve(-1).enable("aaa"); + assertThat(pool.isClosed(), is(false)); + pool.close(); + pool.close(); + + assertThat(pool.isClosed(), is(true)); + assertThat(pool.size(), is(0)); + assertThat(pool.acquire(), nullValue()); + assertThat(pool.reserve(-1), nullValue()); + } + + @Test + public void testClosingCloseable() + { + AtomicBoolean closed = new AtomicBoolean(); + Pool pool = new Pool<>(1,0); + Closeable pooled = () -> closed.set(true); + pool.reserve(-1).enable(pooled); + assertThat(closed.get(), is(false)); + pool.close(); + assertThat(closed.get(), is(true)); + } + + @Test + public void testRemove() + { + Pool pool = new Pool<>(1, 0); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e1 = pool.acquire(); + assertThat(pool.remove(e1), is(true)); + assertThat(pool.remove(e1), is(false)); + assertThat(pool.release(e1), is(false)); + assertThat(pool.acquire(), nullValue()); + assertThrows(NullPointerException.class, () -> pool.remove(null)); + } + + @Test + public void testValuesSize() + { + Pool pool = new Pool<>(2, 0); + + assertThat(pool.size(), is(0)); + assertThat(pool.values().isEmpty(), is(true)); + pool.reserve(-1).enable("aaa"); + pool.reserve(-1).enable("bbb"); + assertThat(pool.values().stream().map(Pool.Entry::getPooled).collect(toList()), equalTo(Arrays.asList("aaa", "bbb"))); + assertThat(pool.size(), is(2)); + } + + @Test + public void testValuesContainsAcquiredEntries() + { + Pool pool = new Pool<>(2, 0); + + pool.reserve(-1).enable("aaa"); + pool.reserve(-1).enable("bbb"); + assertThat(pool.acquire(), notNullValue()); + assertThat(pool.acquire(), notNullValue()); + assertThat(pool.acquire(), nullValue()); + assertThat(pool.values().isEmpty(), is(false)); + } + + @Test + public void testAcquireAt() + { + Pool pool = new Pool<>(2, 0); + + pool.reserve(-1).enable("aaa"); + pool.reserve(-1).enable("bbb"); + + assertThat(pool.acquireAt(2), nullValue()); + assertThat(pool.acquireAt(0), notNullValue()); + assertThat(pool.acquireAt(0), nullValue()); + assertThat(pool.acquireAt(1), notNullValue()); + assertThat(pool.acquireAt(1), nullValue()); + } + + @Test + public void testMaxUsageCount() + { + Pool pool = new Pool<>(1, 0); + pool.setMaxUsageCount(3); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e1 = pool.acquire(); + assertThat(pool.release(e1), is(true)); + e1 = pool.acquire(); + assertThat(pool.release(e1), is(true)); + e1 = pool.acquire(); + assertThat(pool.release(e1), is(false)); + assertThat(pool.acquire(), nullValue()); + assertThat(pool.size(), is(1)); + assertThat(pool.remove(e1), is(true)); + assertThat(pool.remove(e1), is(false)); + assertThat(pool.size(), is(0)); + Pool.Entry e1Copy = e1; + assertThat(pool.release(e1Copy), is(false)); + } + + @Test + public void testMaxMultiplex() + { + Pool pool = new Pool<>(2, 0); + pool.setMaxMultiplex(3); + pool.reserve(-1).enable("aaa"); + pool.reserve(-1).enable("bbb"); + + Pool.Entry e1 = pool.acquire(); + Pool.Entry e2 = pool.acquire(); + Pool.Entry e3 = pool.acquire(); + Pool.Entry e4 = pool.acquire(); + assertThat(e1.getPooled(), equalTo("aaa")); + assertThat(e1, sameInstance(e2)); + assertThat(e1, sameInstance(e3)); + assertThat(e4.getPooled(), equalTo("bbb")); + assertThat(pool.release(e1), is(true)); + Pool.Entry e5 = pool.acquire(); + assertThat(e2, sameInstance(e5)); + Pool.Entry e6 = pool.acquire(); + assertThat(e4, sameInstance(e6)); + } + + @Test + public void testRemoveMultiplexed() + { + Pool pool = new Pool<>(1, 0); + pool.setMaxMultiplex(2); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e1 = pool.acquire(); + Pool.Entry e2 = pool.acquire(); + assertThat(pool.values().stream().findFirst().get().isIdle(), is(false)); + + assertThat(pool.remove(e1), is(false)); + assertThat(pool.values().stream().findFirst().get().isIdle(), is(false)); + assertThat(pool.values().stream().findFirst().get().isClosed(), is(true)); + assertThat(pool.remove(e1), is(true)); + assertThat(pool.size(), is(0)); + + assertThat(pool.remove(e1), is(false)); + + assertThat(pool.release(e1), is(false)); + + assertThat(pool.remove(e1), is(false)); + } + + @Test + public void testMultiplexRemoveThenAcquireThenReleaseRemove() + { + Pool pool = new Pool<>(1, 0); + pool.setMaxMultiplex(2); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e1 = pool.acquire(); + Pool.Entry e2 = pool.acquire(); + + assertThat(pool.remove(e1), is(false)); + assertThat(e1.isClosed(), is(true)); + assertThat(pool.acquire(), nullValue()); + assertThat(pool.release(e2), is(false)); + assertThat(pool.remove(e2), is(true)); + } + + @Test + public void testNonMultiplexRemoveAfterAcquire() + { + Pool pool = new Pool<>(1, 0); + pool.setMaxMultiplex(2); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e1 = pool.acquire(); + assertThat(pool.remove(e1), is(true)); + assertThat(pool.size(), is(0)); + } + + @Test + public void testMultiplexRemoveAfterAcquire() + { + Pool pool = new Pool<>(1, 0); + pool.setMaxMultiplex(2); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e1 = pool.acquire(); + Pool.Entry e2 = pool.acquire(); + + assertThat(pool.remove(e1), is(false)); + assertThat(pool.remove(e2), is(true)); + assertThat(pool.size(), is(0)); + + assertThat(pool.release(e1), is(false)); + assertThat(pool.size(), is(0)); + + Pool.Entry e3 = pool.acquire(); + assertThat(e3, nullValue()); + + assertThat(pool.release(e2), is(false)); + assertThat(pool.size(), is(0)); + } + + @Test + public void testReleaseThenRemoveNonEnabledEntry() + { + Pool pool = new Pool<>(1, 0); + Pool.Entry e = pool.reserve(-1); + assertThat(pool.size(), is(1)); + assertThat(pool.release(e), is(false)); + assertThat(pool.size(), is(1)); + assertThat(pool.remove(e), is(true)); + assertThat(pool.size(), is(0)); + } + + @Test + public void testRemoveNonEnabledEntry() + { + Pool pool = new Pool<>(1, 0); + Pool.Entry e = pool.reserve(-1); + assertThat(pool.size(), is(1)); + assertThat(pool.remove(e), is(true)); + assertThat(pool.size(), is(0)); + } + + @Test + public void testMultiplexMaxUsageReachedAcquireThenRemove() + { + Pool pool = new Pool<>(1, 0); + pool.setMaxMultiplex(2); + pool.setMaxUsageCount(3); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e0 = pool.acquire(); + + Pool.Entry e1 = pool.acquire(); + assertThat(pool.release(e1), is(true)); + Pool.Entry e2 = pool.acquire(); + assertThat(pool.release(e2), is(true)); + assertThat(pool.acquire(), nullValue()); + + assertThat(pool.remove(e0), is(true)); + assertThat(pool.size(), is(0)); + } + + @Test + public void testMultiplexMaxUsageReachedAcquireThenReleaseThenRemove() + { + Pool pool = new Pool<>(1, 0); + pool.setMaxMultiplex(2); + pool.setMaxUsageCount(3); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e0 = pool.acquire(); + + Pool.Entry e1 = pool.acquire(); + assertThat(pool.release(e1), is(true)); + Pool.Entry e2 = pool.acquire(); + assertThat(pool.release(e2), is(true)); + assertThat(pool.acquire(), nullValue()); + + assertThat(pool.release(e0), is(false)); + assertThat(pool.values().stream().findFirst().get().isIdle(), is(true)); + assertThat(pool.values().stream().findFirst().get().isClosed(), is(false)); + assertThat(pool.size(), is(1)); + assertThat(pool.remove(e0), is(true)); + assertThat(pool.size(), is(0)); + } + + @Test + public void testUsageCountAfterReachingMaxMultiplexLimit() + { + Pool pool = new Pool<>(1, 0); + pool.setMaxMultiplex(2); + pool.setMaxUsageCount(10); + pool.reserve(-1).enable("aaa"); + + Pool.Entry e1 = pool.acquire(); + assertThat(e1.getUsageCount(), is(1)); + Pool.Entry e2 = pool.acquire(); + assertThat(e1.getUsageCount(), is(2)); + assertThat(pool.acquire(), nullValue()); + assertThat(e1.getUsageCount(), is(2)); + } + + @Test + public void testConfigLimits() + { + assertThrows(IllegalArgumentException.class, () -> new Pool(1, 0).setMaxMultiplex(0)); + assertThrows(IllegalArgumentException.class, () -> new Pool(1, 0).setMaxMultiplex(-1)); + assertThrows(IllegalArgumentException.class, () -> new Pool(1, 0).setMaxUsageCount(0)); + } +} diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java index 71f50eabf9d..5afbd2a84b5 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/URIUtilCanonicalPathTest.java @@ -114,6 +114,13 @@ public class URIUtilCanonicalPathTest // paths with encoded segments should remain encoded // canonicalPath() is not responsible for decoding characters {"%2e%2e/", "%2e%2e/"}, + {"/%2e%2e/", "/%2e%2e/"}, + + // paths with parameters are not elided + // canonicalPath() is not responsible for decoding characters + {"/foo/.;/bar", "/foo/.;/bar"}, + {"/foo/..;/bar", "/foo/..;/bar"}, + {"/foo/..;/..;/bar", "/foo/..;/..;/bar"}, }; ArrayList ret = new ArrayList<>(); diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/URLEncodedTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/URLEncodedTest.java index d528cf6d171..d3da1f84105 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/URLEncodedTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/URLEncodedTest.java @@ -24,15 +24,21 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.stream.Stream; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; import org.junit.jupiter.api.condition.EnabledIfSystemProperty; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.DynamicTest.dynamicTest; @@ -49,97 +55,97 @@ public class URLEncodedTest tests.add(dynamicTest("Initially not empty", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); assertEquals(0, urlEncoded.size()); })); tests.add(dynamicTest("Not empty after decode(\"\")", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode(""); + UrlEncoded.decodeTo("", urlEncoded, UrlEncoded.ENCODING); assertEquals(0, urlEncoded.size()); })); tests.add(dynamicTest("Simple encode", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode("Name1=Value1"); + UrlEncoded.decodeTo("Name1=Value1", urlEncoded, UrlEncoded.ENCODING); assertEquals(1, urlEncoded.size(), "simple param size"); - assertEquals("Name1=Value1", urlEncoded.encode(), "simple encode"); + assertEquals("Name1=Value1", UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false), "simple encode"); assertEquals("Value1", urlEncoded.getString("Name1"), "simple get"); })); tests.add(dynamicTest("Dangling param", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode("Name2="); + UrlEncoded.decodeTo("Name2=", urlEncoded, UrlEncoded.ENCODING); assertEquals(1, urlEncoded.size(), "dangling param size"); - assertEquals("Name2", urlEncoded.encode(), "dangling encode"); + assertEquals("Name2", UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false), "dangling encode"); assertEquals("", urlEncoded.getString("Name2"), "dangling get"); })); tests.add(dynamicTest("noValue param", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode("Name3"); + UrlEncoded.decodeTo("Name3", urlEncoded, UrlEncoded.ENCODING); assertEquals(1, urlEncoded.size(), "noValue param size"); - assertEquals("Name3", urlEncoded.encode(), "noValue encode"); + assertEquals("Name3", UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false), "noValue encode"); assertEquals("", urlEncoded.getString("Name3"), "noValue get"); })); tests.add(dynamicTest("badly encoded param", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode("Name4=V\u0629lue+4%21"); + UrlEncoded.decodeTo("Name4=V\u0629lue+4%21", urlEncoded, UrlEncoded.ENCODING); assertEquals(1, urlEncoded.size(), "encoded param size"); - assertEquals("Name4=V%D8%A9lue+4%21", urlEncoded.encode(), "encoded encode"); + assertEquals("Name4=V%D8%A9lue+4%21", UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false), "encoded encode"); assertEquals("V\u0629lue 4!", urlEncoded.getString("Name4"), "encoded get"); })); tests.add(dynamicTest("encoded param 1", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode("Name4=Value%2B4%21"); + UrlEncoded.decodeTo("Name4=Value%2B4%21", urlEncoded, UrlEncoded.ENCODING); assertEquals(1, urlEncoded.size(), "encoded param size"); - assertEquals("Name4=Value%2B4%21", urlEncoded.encode(), "encoded encode"); + assertEquals("Name4=Value%2B4%21", UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false), "encoded encode"); assertEquals("Value+4!", urlEncoded.getString("Name4"), "encoded get"); })); tests.add(dynamicTest("encoded param 2", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode("Name4=Value+4%21%20%214"); + UrlEncoded.decodeTo("Name4=Value+4%21%20%214", urlEncoded, UrlEncoded.ENCODING); assertEquals(1, urlEncoded.size(), "encoded param size"); - assertEquals("Name4=Value+4%21+%214", urlEncoded.encode(), "encoded encode"); + assertEquals("Name4=Value+4%21+%214", UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false), "encoded encode"); assertEquals("Value 4! !4", urlEncoded.getString("Name4"), "encoded get"); })); tests.add(dynamicTest("multi param", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode("Name5=aaa&Name6=bbb"); + UrlEncoded.decodeTo("Name5=aaa&Name6=bbb", urlEncoded, UrlEncoded.ENCODING); assertEquals(2, urlEncoded.size(), "multi param size"); - assertTrue(urlEncoded.encode().equals("Name5=aaa&Name6=bbb") || - urlEncoded.encode().equals("Name6=bbb&Name5=aaa"), - "multi encode " + urlEncoded.encode()); + assertTrue(UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false).equals("Name5=aaa&Name6=bbb") || + UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false).equals("Name6=bbb&Name5=aaa"), + "multi encode " + UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false)); assertEquals("aaa", urlEncoded.getString("Name5"), "multi get"); assertEquals("bbb", urlEncoded.getString("Name6"), "multi get"); })); tests.add(dynamicTest("multiple value encoded", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode("Name7=aaa&Name7=b%2Cb&Name7=ccc"); - assertEquals("Name7=aaa&Name7=b%2Cb&Name7=ccc", urlEncoded.encode(), "multi encode"); + UrlEncoded.decodeTo("Name7=aaa&Name7=b%2Cb&Name7=ccc", urlEncoded, UrlEncoded.ENCODING); + assertEquals("Name7=aaa&Name7=b%2Cb&Name7=ccc", UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false), "multi encode"); assertEquals("aaa,b,b,ccc", urlEncoded.getString("Name7"), "list get all"); assertEquals("aaa", urlEncoded.getValues("Name7").get(0), "list get"); assertEquals("b,b", urlEncoded.getValues("Name7").get(1), "list get"); @@ -148,11 +154,11 @@ public class URLEncodedTest tests.add(dynamicTest("encoded param", () -> { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); urlEncoded.clear(); - urlEncoded.decode("Name8=xx%2C++yy++%2Czz"); + UrlEncoded.decodeTo("Name8=xx%2C++yy++%2Czz", urlEncoded, UrlEncoded.ENCODING); assertEquals(1, urlEncoded.size(), "encoded param size"); - assertEquals("Name8=xx%2C++yy++%2Czz", urlEncoded.encode(), "encoded encode"); + assertEquals("Name8=xx%2C++yy++%2Czz", UrlEncoded.encode(urlEncoded,UrlEncoded.ENCODING, false), "encoded encode"); assertEquals("xx, yy ,zz", urlEncoded.getString("Name8"), "encoded get"); })); @@ -219,7 +225,7 @@ public class URLEncodedTest { try (ByteArrayInputStream in3 = new ByteArrayInputStream("name=libell%E9".getBytes(StringUtil.__ISO_8859_1))) { - MultiMap m3 = new MultiMap(); + MultiMap m3 = new MultiMap<>(); Charset nullCharset = null; // use the one from the system property UrlEncoded.decodeTo(in3, m3, nullCharset, -1, -1); assertEquals("libell\u00E9", m3.getString("name"), "stream name"); @@ -230,11 +236,11 @@ public class URLEncodedTest public void testUtf8() throws Exception { - UrlEncoded urlEncoded = new UrlEncoded(); + MultiMap urlEncoded = new MultiMap<>(); assertEquals(0, urlEncoded.size(), "Empty"); urlEncoded.clear(); - urlEncoded.decode("text=%E0%B8%9F%E0%B8%AB%E0%B8%81%E0%B8%A7%E0%B8%94%E0%B8%B2%E0%B9%88%E0%B8%81%E0%B8%9F%E0%B8%A7%E0%B8%AB%E0%B8%AA%E0%B8%94%E0%B8%B2%E0%B9%88%E0%B8%AB%E0%B8%9F%E0%B8%81%E0%B8%A7%E0%B8%94%E0%B8%AA%E0%B8%B2%E0%B8%9F%E0%B8%81%E0%B8%AB%E0%B8%A3%E0%B8%94%E0%B9%89%E0%B8%9F%E0%B8%AB%E0%B8%99%E0%B8%81%E0%B8%A3%E0%B8%94%E0%B8%B5&Action=Submit"); + UrlEncoded.decodeTo("text=%E0%B8%9F%E0%B8%AB%E0%B8%81%E0%B8%A7%E0%B8%94%E0%B8%B2%E0%B9%88%E0%B8%81%E0%B8%9F%E0%B8%A7%E0%B8%AB%E0%B8%AA%E0%B8%94%E0%B8%B2%E0%B9%88%E0%B8%AB%E0%B8%9F%E0%B8%81%E0%B8%A7%E0%B8%94%E0%B8%AA%E0%B8%B2%E0%B8%9F%E0%B8%81%E0%B8%AB%E0%B8%A3%E0%B8%94%E0%B9%89%E0%B8%9F%E0%B8%AB%E0%B8%99%E0%B8%81%E0%B8%A3%E0%B8%94%E0%B8%B5&Action=Submit", urlEncoded, UrlEncoded.ENCODING); String hex = "E0B89FE0B8ABE0B881E0B8A7E0B894E0B8B2E0B988E0B881E0B89FE0B8A7E0B8ABE0B8AAE0B894E0B8B2E0B988E0B8ABE0B89FE0B881E0B8A7E0B894E0B8AAE0B8B2E0B89FE0B881E0B8ABE0B8A3E0B894E0B989E0B89FE0B8ABE0B899E0B881E0B8A3E0B894E0B8B5"; String expected = new String(TypeUtil.fromHexString(hex), "utf-8"); @@ -245,8 +251,8 @@ public class URLEncodedTest public void testUtf8MultiByteCodePoint() { String input = "text=test%C3%A4"; - UrlEncoded urlEncoded = new UrlEncoded(); - urlEncoded.decode(input); + MultiMap urlEncoded = new MultiMap<>(); + UrlEncoded.decodeTo(input, urlEncoded, UrlEncoded.ENCODING); // http://www.ltg.ed.ac.uk/~richard/utf-8.cgi?input=00e4&mode=hex // Should be "testä" @@ -255,4 +261,50 @@ public class URLEncodedTest String expected = "test\u00e4"; assertThat(urlEncoded.getString("text"), is(expected)); } + + public static Stream invalidTestData() + { + ArrayList data = new ArrayList<>(); + data.add(Arguments.of("Name=xx%zzyy", UTF_8, IllegalArgumentException.class)); + data.add(Arguments.of("Name=%FF%FF%FF", UTF_8, Utf8Appendable.NotUtf8Exception.class)); + data.add(Arguments.of("Name=%EF%EF%EF", UTF_8, Utf8Appendable.NotUtf8Exception.class)); + data.add(Arguments.of("Name=%E%F%F", UTF_8, IllegalArgumentException.class)); + data.add(Arguments.of("Name=x%", UTF_8, Utf8Appendable.NotUtf8Exception.class)); + data.add(Arguments.of("Name=x%2", UTF_8, Utf8Appendable.NotUtf8Exception.class)); + data.add(Arguments.of("Name=xxx%", UTF_8, Utf8Appendable.NotUtf8Exception.class)); + data.add(Arguments.of("name=X%c0%afZ", UTF_8, Utf8Appendable.NotUtf8Exception.class)); + return data.stream(); + } + + @ParameterizedTest + @MethodSource("invalidTestData") + public void testInvalidDecode(String inputString, Charset charset, Class expectedThrowable) + { + assertThrows(expectedThrowable, () -> + { + UrlEncoded.decodeTo(inputString, new MultiMap<>(), charset); + }); + } + + @ParameterizedTest + @MethodSource("invalidTestData") + public void testInvalidDecodeUtf8ToMap(String inputString, Charset charset, Class expectedThrowable) + { + assertThrows(expectedThrowable, () -> + { + MultiMap map = new MultiMap<>(); + UrlEncoded.decodeUtf8To(inputString, map); + }); + } + + @ParameterizedTest + @MethodSource("invalidTestData") + public void testInvalidDecodeTo(String inputString, Charset charset, Class expectedThrowable) + { + assertThrows(expectedThrowable, () -> + { + MultiMap map = new MultiMap<>(); + UrlEncoded.decodeTo(inputString, map, charset); + }); + } } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/UrlEncodedInvalidEncodingTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/UrlEncodedInvalidEncodingTest.java deleted file mode 100644 index 4df751118c9..00000000000 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/UrlEncodedInvalidEncodingTest.java +++ /dev/null @@ -1,80 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under -// the terms of the Eclipse Public License 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0 -// -// This Source Code may also be made available under the following -// Secondary Licenses when the conditions for such availability set -// forth in the Eclipse Public License, v. 2.0 are satisfied: -// the Apache License v2.0 which is available at -// https://www.apache.org/licenses/LICENSE-2.0 -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.util; - -import java.nio.charset.Charset; -import java.util.ArrayList; -import java.util.stream.Stream; - -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; - -import static java.nio.charset.StandardCharsets.UTF_8; -import static org.junit.jupiter.api.Assertions.assertThrows; - -public class UrlEncodedInvalidEncodingTest -{ - public static Stream data() - { - ArrayList data = new ArrayList<>(); - data.add(Arguments.of("Name=xx%zzyy", UTF_8, IllegalArgumentException.class)); - data.add(Arguments.of("Name=%FF%FF%FF", UTF_8, Utf8Appendable.NotUtf8Exception.class)); - data.add(Arguments.of("Name=%EF%EF%EF", UTF_8, Utf8Appendable.NotUtf8Exception.class)); - data.add(Arguments.of("Name=%E%F%F", UTF_8, IllegalArgumentException.class)); - data.add(Arguments.of("Name=x%", UTF_8, Utf8Appendable.NotUtf8Exception.class)); - data.add(Arguments.of("Name=x%2", UTF_8, Utf8Appendable.NotUtf8Exception.class)); - data.add(Arguments.of("Name=xxx%", UTF_8, Utf8Appendable.NotUtf8Exception.class)); - data.add(Arguments.of("name=X%c0%afZ", UTF_8, Utf8Appendable.NotUtf8Exception.class)); - return data.stream(); - } - - @ParameterizedTest - @MethodSource("data") - public void testDecode(String inputString, Charset charset, Class expectedThrowable) - { - assertThrows(expectedThrowable, () -> - { - UrlEncoded urlEncoded = new UrlEncoded(); - urlEncoded.decode(inputString, charset); - }); - } - - @ParameterizedTest - @MethodSource("data") - public void testDecodeUtf8ToMap(String inputString, Charset charset, Class expectedThrowable) - { - assertThrows(expectedThrowable, () -> - { - MultiMap map = new MultiMap<>(); - UrlEncoded.decodeUtf8To(inputString, map); - }); - } - - @ParameterizedTest - @MethodSource("data") - public void testDecodeTo(String inputString, Charset charset, Class expectedThrowable) - { - assertThrows(expectedThrowable, () -> - { - MultiMap map = new MultiMap<>(); - UrlEncoded.decodeTo(inputString, map, charset); - }); - } -} diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java index 284d1332247..a494972dc14 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java @@ -1314,7 +1314,6 @@ public class WebAppContext extends ServletContextHandler implements WebAppClassL setClassLoader(null); } - setAvailable(true); _unavailableException = null; } } diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java index 44c07a2815b..57a7548e078 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java @@ -40,8 +40,8 @@ import org.slf4j.LoggerFactory; public abstract class JavaxWebSocketContainer extends ContainerLifeCycle implements javax.websocket.WebSocketContainer { private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketContainer.class); - private final SessionTracker sessionTracker = new SessionTracker(); private final List sessionListeners = new ArrayList<>(); + protected final SessionTracker sessionTracker = new SessionTracker(); protected final Configuration.ConfigurationCustomizer defaultCustomizer = new Configuration.ConfigurationCustomizer(); protected final WebSocketComponents components; diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java index 600540259a3..ac4093760a6 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java @@ -18,26 +18,26 @@ package org.eclipse.jetty.websocket.javax.common; -import java.io.IOException; import java.util.Collections; +import java.util.HashSet; import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArraySet; import javax.websocket.CloseReason; import javax.websocket.Session; import org.eclipse.jetty.util.component.AbstractLifeCycle; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.eclipse.jetty.util.component.Graceful; -public class SessionTracker extends AbstractLifeCycle implements JavaxWebSocketSessionListener +public class SessionTracker extends AbstractLifeCycle implements JavaxWebSocketSessionListener, Graceful { - private static final Logger LOG = LoggerFactory.getLogger(SessionTracker.class); - - private final CopyOnWriteArraySet sessions = new CopyOnWriteArraySet<>(); + private final Set sessions = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private boolean isShutdown = false; public Set getSessions() { - return Collections.unmodifiableSet(sessions); + return Set.copyOf(sessions); } @Override @@ -52,22 +52,40 @@ public class SessionTracker extends AbstractLifeCycle implements JavaxWebSocketS sessions.remove(session); } + @Override + protected void doStart() throws Exception + { + isShutdown = false; + super.doStart(); + } + @Override protected void doStop() throws Exception { - for (Session session : sessions) + sessions.clear(); + super.doStop(); + } + + @Override + public CompletableFuture shutdown() + { + isShutdown = true; + return Graceful.shutdown(() -> { - try + for (Session session : sessions) { + if (Thread.interrupted()) + break; + // GOING_AWAY is abnormal close status so it will hard close connection after sent. session.close(new CloseReason(CloseReason.CloseCodes.GOING_AWAY, "Container being shut down")); } - catch (IOException e) - { - LOG.trace("IGNORED", e); - } - } + }); + } - super.doStop(); + @Override + public boolean isShutdown() + { + return isShutdown; } } diff --git a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EchoSocket.java b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EchoSocket.java new file mode 100644 index 00000000000..c18d8b665a4 --- /dev/null +++ b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EchoSocket.java @@ -0,0 +1,43 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.websocket.javax.tests; + +import java.io.IOException; +import java.nio.ByteBuffer; +import javax.websocket.ClientEndpoint; +import javax.websocket.server.ServerEndpoint; + +@ServerEndpoint("/") +@ClientEndpoint +public class EchoSocket extends EventSocket +{ + @Override + public void onMessage(String message) throws IOException + { + super.onMessage(message); + session.getBasicRemote().sendText(message); + } + + @Override + public void onMessage(ByteBuffer message) throws IOException + { + super.onMessage(message); + session.getBasicRemote().sendBinary(message); + } +} diff --git a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java index a8b7d1a5fe9..a69db90ba45 100644 --- a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java +++ b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java @@ -98,23 +98,4 @@ public class EventSocket error = cause; errorLatch.countDown(); } - - @ServerEndpoint("/") - @ClientEndpoint - public static class EchoSocket extends EventSocket - { - @Override - public void onMessage(String message) throws IOException - { - super.onMessage(message); - session.getBasicRemote().sendText(message); - } - - @Override - public void onMessage(ByteBuffer message) throws IOException - { - super.onMessage(message); - session.getBasicRemote().sendBinary(message); - } - } } diff --git a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java new file mode 100644 index 00000000000..b2745345484 --- /dev/null +++ b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java @@ -0,0 +1,133 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.websocket.javax.tests; + +import java.net.URI; +import java.util.Objects; +import java.util.concurrent.TimeUnit; +import javax.websocket.CloseReason; +import javax.websocket.EndpointConfig; +import javax.websocket.Session; +import javax.websocket.server.ServerEndpoint; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.util.BlockingArrayQueue; +import org.eclipse.jetty.util.component.Graceful; +import org.eclipse.jetty.websocket.javax.client.internal.JavaxWebSocketClientContainer; +import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class GracefulCloseTest +{ + private static final BlockingArrayQueue serverEndpoints = new BlockingArrayQueue<>(); + private Server server; + private URI serverUri; + private JavaxWebSocketClientContainer client; + + @BeforeEach + public void before() throws Exception + { + server = new Server(); + ServerConnector connector = new ServerConnector(server); + server.addConnector(connector); + + ServletContextHandler contextHandler = new ServletContextHandler(); + contextHandler.setContextPath("/"); + server.setHandler(contextHandler); + JavaxWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> + container.addEndpoint(ServerSocket.class)); + server.start(); + serverUri = WSURI.toWebsocket(server.getURI()); + + // StopTimeout is necessary for the websocket server sessions to gracefully close. + server.setStopTimeout(1000); + + client = new JavaxWebSocketClientContainer(); + client.start(); + } + + @AfterEach + public void after() throws Exception + { + client.stop(); + server.stop(); + } + + @ServerEndpoint("/") + public static class ServerSocket extends EchoSocket + { + @Override + public void onOpen(Session session, EndpointConfig endpointConfig) + { + serverEndpoints.add(this); + super.onOpen(session, endpointConfig); + } + } + + @Test + public void testClientStop() throws Exception + { + EventSocket clientEndpoint = new EventSocket(); + client.connectToServer(clientEndpoint, serverUri); + EventSocket serverEndpoint = Objects.requireNonNull(serverEndpoints.poll(5, TimeUnit.SECONDS)); + + // There is no API for a Javax WebSocketContainer stop timeout. + Graceful.shutdown(client).get(5, TimeUnit.SECONDS); + client.stop(); + + // Check that the client endpoint was closed with the correct status code and no error. + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeReason.getCloseCode(), is(CloseReason.CloseCodes.GOING_AWAY)); + assertNull(clientEndpoint.error); + + // Check that the server endpoint was closed with the correct status code and no error. + assertTrue(serverEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(serverEndpoint.closeReason.getCloseCode(), is(CloseReason.CloseCodes.GOING_AWAY)); + assertNull(serverEndpoint.error); + } + + @Test + public void testServerStop() throws Exception + { + EventSocket clientEndpoint = new EventSocket(); + client.connectToServer(clientEndpoint, serverUri); + EventSocket serverEndpoint = Objects.requireNonNull(serverEndpoints.poll(5, TimeUnit.SECONDS)); + + server.stop(); + + // Check that the client endpoint was closed with the correct status code and no error. + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeReason.getCloseCode(), is(CloseReason.CloseCodes.GOING_AWAY)); + assertNull(clientEndpoint.error); + + // Check that the server endpoint was closed with the correct status code and no error. + assertTrue(serverEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(serverEndpoint.closeReason.getCloseCode(), is(CloseReason.CloseCodes.GOING_AWAY)); + assertNull(serverEndpoint.error); + } +} diff --git a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/coders/EncoderLifeCycleTest.java b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/coders/EncoderLifeCycleTest.java index cc03423accd..88657bbbd74 100644 --- a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/coders/EncoderLifeCycleTest.java +++ b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/coders/EncoderLifeCycleTest.java @@ -40,7 +40,7 @@ import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketSession; import org.eclipse.jetty.websocket.javax.common.encoders.AvailableEncoders; import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer; -import org.eclipse.jetty.websocket.javax.tests.EventSocket.EchoSocket; +import org.eclipse.jetty.websocket.javax.tests.EchoSocket; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.params.ParameterizedTest; diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index da4760d655f..9abdbbed8af 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java @@ -29,6 +29,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import org.eclipse.jetty.client.HttpClient; @@ -38,6 +39,7 @@ import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.util.DecoratedObjectFactory; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.ShutdownThread; import org.eclipse.jetty.websocket.api.Session; @@ -68,6 +70,7 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketPoli private final Configuration.ConfigurationCustomizer configurationCustomizer = new Configuration.ConfigurationCustomizer(); private final WebSocketComponents components = new WebSocketComponents(); private boolean stopAtShutdown = false; + private long _stopTimeout = Long.MAX_VALUE; /** * Instantiate a WebSocketClient with defaults @@ -388,11 +391,33 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketPoli stopAtShutdown = stop; } + /** + * The timeout to allow all remaining open Sessions to be closed gracefully using the close code {@link org.eclipse.jetty.websocket.api.StatusCode#SHUTDOWN}. + * @param stopTimeout the time in ms to wait for the graceful close, use a value less than or equal to 0 to not gracefully close. + */ + public void setStopTimeout(long stopTimeout) + { + _stopTimeout = stopTimeout; + } + + public long getStopTimeout() + { + return _stopTimeout; + } + public boolean isStopAtShutdown() { return stopAtShutdown; } + @Override + protected void doStop() throws Exception + { + if (getStopTimeout() > 0) + Graceful.shutdown(this).get(getStopTimeout(), TimeUnit.MILLISECONDS); + super.doStop(); + } + @Override public String toString() { diff --git a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java index e963800ae9f..5a7d4ea44b5 100644 --- a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java +++ b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java @@ -19,21 +19,28 @@ package org.eclipse.jetty.websocket.common; import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.StatusCode; import org.eclipse.jetty.websocket.api.WebSocketSessionListener; -public class SessionTracker extends AbstractLifeCycle implements WebSocketSessionListener +public class SessionTracker extends AbstractLifeCycle implements WebSocketSessionListener, Graceful { - private List sessions = new CopyOnWriteArrayList<>(); + private final Set sessions = Collections.newSetFromMap(new ConcurrentHashMap<>()); + private boolean isShutdown = false; public Collection getSessions() { - return sessions; + return Set.copyOf(sessions); } @Override @@ -48,15 +55,40 @@ public class SessionTracker extends AbstractLifeCycle implements WebSocketSessio sessions.remove(session); } + @Override + protected void doStart() throws Exception + { + isShutdown = false; + super.doStart(); + } + @Override protected void doStop() throws Exception { - for (Session session : sessions) - { - // SHUTDOWN is abnormal close status so it will hard close connection after sent. - session.close(StatusCode.SHUTDOWN, "Container being shut down"); - } - + sessions.clear(); super.doStop(); } + + @Override + public CompletableFuture shutdown() + { + isShutdown = true; + return Graceful.shutdown(() -> + { + for (Session session : sessions) + { + if (Thread.interrupted()) + break; + + // SHUTDOWN is abnormal close status so it will hard close connection after sent. + session.close(StatusCode.SHUTDOWN, "Container being shut down"); + } + }); + } + + @Override + public boolean isShutdown() + { + return isShutdown; + } } diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java index ed0c40b07fa..5eadef61ef3 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java @@ -119,6 +119,7 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements frameHandlerFactory = factory; addSessionListener(sessionTracker); + addBean(sessionTracker); } public void addMapping(String pathSpec, JettyWebSocketCreator creator) diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java new file mode 100644 index 00000000000..d3157d0f283 --- /dev/null +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java @@ -0,0 +1,114 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.websocket.tests; + +import java.net.URI; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.websocket.api.StatusCode; +import org.eclipse.jetty.websocket.api.util.WSURI; +import org.eclipse.jetty.websocket.client.WebSocketClient; +import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class GracefulCloseTest +{ + private final EventSocket serverEndpoint = new EchoSocket(); + private Server server; + private URI serverUri; + private WebSocketClient client; + + @BeforeEach + public void before() throws Exception + { + server = new Server(); + ServerConnector connector = new ServerConnector(server); + server.addConnector(connector); + + ServletContextHandler contextHandler = new ServletContextHandler(); + contextHandler.setContextPath("/"); + server.setHandler(contextHandler); + JettyWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> + container.addMapping("/", ((req, resp) -> serverEndpoint))); + server.start(); + serverUri = WSURI.toWebsocket(server.getURI()); + + // StopTimeout is necessary for the websocket server sessions to gracefully close. + server.setStopTimeout(1000); + + client = new WebSocketClient(); + client.setStopTimeout(1000); + client.start(); + } + + @AfterEach + public void after() throws Exception + { + client.stop(); + server.stop(); + } + + @Test + public void testClientStop() throws Exception + { + EventSocket clientEndpoint = new EventSocket(); + client.connect(clientEndpoint, serverUri).get(5, TimeUnit.SECONDS); + + client.stop(); + + // Check that the client endpoint was closed with the correct status code and no error. + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeCode, is(StatusCode.SHUTDOWN)); + assertNull(clientEndpoint.error); + + // Check that the server endpoint was closed with the correct status code and no error. + assertTrue(serverEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(serverEndpoint.closeCode, is(StatusCode.SHUTDOWN)); + assertNull(serverEndpoint.error); + } + + @Test + public void testServerStop() throws Exception + { + EventSocket clientEndpoint = new EventSocket(); + client.connect(clientEndpoint, serverUri).get(5, TimeUnit.SECONDS); + + server.stop(); + + // Check that the client endpoint was closed with the correct status code and no error. + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeCode, is(StatusCode.SHUTDOWN)); + assertNull(clientEndpoint.error); + + // Check that the server endpoint was closed with the correct status code and no error. + assertTrue(serverEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(serverEndpoint.closeCode, is(StatusCode.SHUTDOWN)); + assertNull(serverEndpoint.error); + } +} diff --git a/tests/jetty-jmh/pom.xml b/tests/jetty-jmh/pom.xml index edf3878dfd9..200943c79f1 100644 --- a/tests/jetty-jmh/pom.xml +++ b/tests/jetty-jmh/pom.xml @@ -106,6 +106,11 @@ jetty-slf4j-impl test + + org.eclipse.jetty + jetty-client + ${project.version} + org.eclipse.jetty.toolchain jetty-test-helper diff --git a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/client/jmh/ConnectionPoolsBenchmark.java b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/client/jmh/ConnectionPoolsBenchmark.java new file mode 100644 index 00000000000..e728bd8d196 --- /dev/null +++ b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/client/jmh/ConnectionPoolsBenchmark.java @@ -0,0 +1,174 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.client.jmh; + +import java.net.URI; +import java.util.ArrayList; +import java.util.concurrent.ThreadLocalRandom; + +import org.eclipse.jetty.client.ConnectionPool; +import org.eclipse.jetty.client.DuplexConnectionPool; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.HttpConversation; +import org.eclipse.jetty.client.HttpDestination; +import org.eclipse.jetty.client.HttpExchange; +import org.eclipse.jetty.client.HttpRequest; +import org.eclipse.jetty.client.MultiplexConnectionPool; +import org.eclipse.jetty.client.Origin; +import org.eclipse.jetty.client.RoundRobinConnectionPool; +import org.eclipse.jetty.client.api.Connection; +import org.eclipse.jetty.client.api.Request; +import org.eclipse.jetty.client.api.Response; +import org.eclipse.jetty.util.Attachable; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.Promise; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.Param; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.infra.Blackhole; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +@State(Scope.Benchmark) +public class ConnectionPoolsBenchmark +{ + private ConnectionPool pool; + + @Param({"round-robin", "cached/multiplex", "uncached/multiplex", "cached/duplex", "uncached/duplex"}) + public static String POOL_TYPE; + + @Setup + public void setUp() throws Exception + { + HttpClient httpClient = new HttpClient() + { + @Override + protected void newConnection(HttpDestination destination, Promise promise) + { + promise.succeeded(new MockConnection()); + } + }; + HttpDestination httpDestination = new HttpDestination(httpClient, new Origin("http", "localhost", 8080)) + { + }; + + HttpConversation httpConversation = new HttpConversation(); + HttpRequest httpRequest = new HttpRequest(httpClient, httpConversation, new URI("http://localhost:8080")) {}; + HttpExchange httpExchange = new HttpExchange(httpDestination, httpRequest, new ArrayList<>()); + httpDestination.getHttpExchanges().add(httpExchange); + + int initialConnections = 12; + int maxConnections = 100; + switch (POOL_TYPE) + { + case "uncached/duplex": + pool = new DuplexConnectionPool(httpDestination, maxConnections, false, Callback.NOOP); + pool.preCreateConnections(initialConnections).get(); + break; + case "cached/duplex": + pool = new DuplexConnectionPool(httpDestination, maxConnections, true, Callback.NOOP); + pool.preCreateConnections(initialConnections).get(); + break; + case "uncached/multiplex": + pool = new MultiplexConnectionPool(httpDestination, maxConnections,false, Callback.NOOP, 12); + pool.preCreateConnections(initialConnections).get(); + break; + case "cached/multiplex": + pool = new MultiplexConnectionPool(httpDestination, maxConnections,true, Callback.NOOP, 12); + pool.preCreateConnections(initialConnections).get(); + break; + case "round-robin": + pool = new RoundRobinConnectionPool(httpDestination, maxConnections, Callback.NOOP); + pool.preCreateConnections(maxConnections).get(); + break; + default: + throw new AssertionError("Unknown pool type: " + POOL_TYPE); + } + } + + @TearDown + public void tearDown() + { + pool.close(); + pool = null; + } + + @Benchmark + public void testPool() + { + Connection connection = pool.acquire(true); + if (connection == null && !POOL_TYPE.equals("round-robin")) + throw new AssertionError("from thread " + Thread.currentThread().getName()); + Blackhole.consumeCPU(ThreadLocalRandom.current().nextInt(10, 20)); + if (connection != null) + pool.release(connection); + } + + public static void main(String[] args) throws RunnerException + { + Options opt = new OptionsBuilder() + .include(ConnectionPoolsBenchmark.class.getSimpleName()) + .warmupIterations(3) + .measurementIterations(3) + .forks(1) + .threads(12) + //.addProfiler(LinuxPerfProfiler.class) + .build(); + + new Runner(opt).run(); + } + + static class MockConnection implements Connection, Attachable + { + private Object attachment; + + @Override + public void close() + { + } + + @Override + public boolean isClosed() + { + return false; + } + + @Override + public void send(Request request, Response.CompleteListener listener) + { + } + + @Override + public void setAttachment(Object obj) + { + this.attachment = obj; + } + + @Override + public Object getAttachment() + { + return attachment; + } + } +} diff --git a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/SessionDump.java b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/SessionDump.java index 86fdcca40df..67bd337b7c6 100644 --- a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/SessionDump.java +++ b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/SessionDump.java @@ -79,7 +79,7 @@ public class SessionDump extends HttpServlet session = request.getSession(true); session.setAttribute("test", "value"); session.setAttribute("obj", new ObjectAttributeValue(System.currentTimeMillis())); - session.setAttribute("WEBCL", new MultiMap()); + session.setAttribute("WEBCL", new MultiMap<>()); } else if (session != null) { @@ -137,7 +137,7 @@ public class SessionDump extends HttpServlet else { if (session.getAttribute("WEBCL") == null) - session.setAttribute("WEBCL", new MultiMap()); + session.setAttribute("WEBCL", new MultiMap<>()); try { out.println("ID: " + session.getId() + "
");