From 8d69fc41a7f8c26fcd90fc0f7300f81eaede06be Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 27 Jul 2020 23:14:05 +0200 Subject: [PATCH] Fixes #5083 - Convert synchronized usages to AutoLock. * Replaced relevant usages of synchronized with AutoLock. * Made AutoLock serializable since classes that use it may be stored in the HttpSession. * Added convenience methods to AutoLock to execute lambdas with the lock held. * Introduced AutoLock.WithCondition to use a Lock and a Condition together. Signed-off-by: Simone Bordet --- .../annotations/AnnotationIntrospector.java | 16 +- .../org/eclipse/jetty/client/HttpChannel.java | 11 +- .../eclipse/jetty/client/HttpConnection.java | 8 +- .../eclipse/jetty/client/HttpExchange.java | 41 +- .../eclipse/jetty/client/HttpReceiver.java | 11 +- .../jetty/client/MultiplexConnectionPool.java | 40 +- .../client/RoundRobinConnectionPool.java | 30 +- .../client/util/AsyncRequestContent.java | 20 +- .../jetty/deploy/DeploymentManager.java | 2 +- .../client/http/HttpConnectionOverFCGI.java | 9 +- .../eclipse/jetty/fcgi/generator/Flusher.java | 12 +- .../org/eclipse/jetty/http/MimeTypes.java | 2 +- .../jetty/http2/client/RawHTTP2ProxyTest.java | 34 +- .../eclipse/jetty/http2/HTTP2Connection.java | 12 +- .../org/eclipse/jetty/http2/HTTP2Flusher.java | 24 +- .../org/eclipse/jetty/http2/HTTP2Session.java | 16 +- .../org/eclipse/jetty/http2/HTTP2Stream.java | 41 +- .../jetty/http2/HTTP2StreamEndPoint.java | 25 +- .../client/http/HttpReceiverOverHTTP2.java | 17 +- .../http2/server/HTTP2ServerConnection.java | 20 +- .../http2/server/HttpTransportOverHTTP2.java | 18 +- .../eclipse/jetty/io/ByteArrayEndPoint.java | 24 +- .../org/eclipse/jetty/io/ManagedSelector.java | 19 +- .../jetty/io/SocketChannelEndPoint.java | 8 +- .../eclipse/jetty/io/WriterOutputStream.java | 7 +- .../eclipse/jetty/io/ssl/SslConnection.java | 33 +- .../eclipse/jetty/io/SslConnectionTest.java | 2 +- .../org/eclipse/jetty/jaas/spi/UserInfo.java | 11 +- .../eclipse/jetty/jndi/ContextFactory.java | 42 +- .../osgi/boot/utils/OSGiClassLoader.java | 55 +- .../authentication/DigestAuthenticator.java | 4 +- .../jetty/server/AbstractConnector.java | 22 +- .../eclipse/jetty/server/AcceptRateLimit.java | 19 +- .../jetty/server/AsyncRequestLogWriter.java | 2 +- .../eclipse/jetty/server/ConnectionLimit.java | 31 +- .../jetty/server/CustomRequestLog.java | 2 +- .../jetty/server/HttpChannelState.java | 165 ++--- .../org/eclipse/jetty/server/HttpInput.java | 61 +- .../org/eclipse/jetty/server/HttpOutput.java | 56 +- .../server/MultiPartFormInputStream.java | 41 +- .../jetty/server/RequestLogWriter.java | 36 +- .../eclipse/jetty/server/ResourceService.java | 25 +- .../eclipse/jetty/server/ShutdownMonitor.java | 65 +- .../jetty/server/Slf4jRequestLogWriter.java | 2 +- .../jetty/server/handler/ContextHandler.java | 201 +++--- .../session/DefaultSessionIdManager.java | 10 +- .../jetty/server/session/HouseKeeper.java | 10 +- .../eclipse/jetty/server/session/Session.java | 105 ++-- .../jetty/server/session/SessionHandler.java | 1 + .../server/ServerConnectorTimeoutTest.java | 2 +- .../org/eclipse/jetty/servlet/BaseHolder.java | 47 +- .../eclipse/jetty/servlet/FilterHolder.java | 22 +- .../org/eclipse/jetty/servlet/Holder.java | 12 +- .../org/eclipse/jetty/servlet/Invoker.java | 5 +- .../eclipse/jetty/servlet/ListenerHolder.java | 19 +- .../eclipse/jetty/servlet/ServletHandler.java | 401 ++++++------ .../eclipse/jetty/servlet/ServletHolder.java | 71 ++- .../org/eclipse/jetty/servlets/DoSFilter.java | 6 +- .../jetty/servlets/EventSourceServlet.java | 20 +- .../jetty/util/ajax/JSONDateConvertor.java | 4 +- .../jetty/util/RolloverFileOutputStream.java | 46 +- .../java/org/eclipse/jetty/util/Scanner.java | 319 +++++----- .../org/eclipse/jetty/util/StringUtil.java | 13 +- .../java/org/eclipse/jetty/util/URIUtil.java | 33 +- .../org/eclipse/jetty/util/UrlEncoded.java | 581 +++++++++--------- .../util/component/AbstractLifeCycle.java | 7 +- .../util/component/AttributeContainerMap.java | 41 +- .../jetty/util/resource/JarFileResource.java | 153 ++--- .../jetty/util/resource/JarResource.java | 38 +- .../jetty/util/resource/URLResource.java | 97 +-- .../jetty/util/security/Credential.java | 36 +- .../jetty/util/ssl/SslContextFactory.java | 17 +- .../jetty/util/statistic/RateStatistic.java | 27 +- .../eclipse/jetty/util/thread/AutoLock.java | 124 +++- .../jetty/util/thread/QueuedThreadPool.java | 10 +- .../jetty/util/thread/ShutdownThread.java | 60 +- .../util/thread/strategy/EatWhatYouKill.java | 24 +- .../eclipse/jetty/webapp/Configurations.java | 84 +-- .../org/eclipse/jetty/webapp/MetaData.java | 51 +- .../jetty/webapp/ConfigurationsTest.java | 4 +- .../jetty/webapp/WebAppContextTest.java | 8 +- .../websocket/core/internal/FrameFlusher.java | 21 +- .../core/internal/FrameSequence.java | 4 +- .../core/internal/TransformingFlusher.java | 15 +- .../core/internal/WebSocketConnection.java | 25 +- .../core/internal/WebSocketSessionState.java | 28 +- .../common/JavaxWebSocketFrameHandler.java | 41 +- .../javax/common/JavaxWebSocketSession.java | 2 +- .../websocket/client/WebSocketClient.java | 2 +- .../common/JettyWebSocketFrameHandler.java | 17 +- .../util/messages/MessageInputStream.java | 13 +- .../util/messages/MessageOutputStream.java | 27 +- .../eclipse/jetty/xml/XmlConfiguration.java | 3 +- .../java/org/eclipse/jetty/xml/XmlParser.java | 91 ++- .../jetty/http/tools/HttpTesterTest.java | 2 +- .../util/jmh/DateCacheSimpleDateFormat.java | 37 +- .../main/java/com/acme/DispatchServlet.java | 5 - .../src/main/java/com/acme/Dump.java | 2 +- .../src/main/java/com/acme/RegTest.java | 5 - 99 files changed, 2044 insertions(+), 2146 deletions(-) diff --git a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationIntrospector.java b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationIntrospector.java index 4ffe33c9cc2..aed655445f3 100644 --- a/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationIntrospector.java +++ b/jetty-annotations/src/main/java/org/eclipse/jetty/annotations/AnnotationIntrospector.java @@ -28,6 +28,7 @@ import java.util.Set; import org.eclipse.jetty.servlet.BaseHolder; import org.eclipse.jetty.servlet.Source.Origin; import org.eclipse.jetty.util.resource.Resource; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.webapp.WebAppContext; import org.eclipse.jetty.webapp.WebDescriptor; import org.slf4j.Logger; @@ -41,6 +42,8 @@ import org.slf4j.LoggerFactory; public class AnnotationIntrospector { private static final Logger LOG = LoggerFactory.getLogger(AnnotationIntrospector.class); + + private final AutoLock _lock = new AutoLock(); private final Set> _introspectedClasses = new HashSet<>(); private final List _handlers = new ArrayList(); private final WebAppContext _context; @@ -195,14 +198,13 @@ public class AnnotationIntrospector Class clazz = o.getClass(); - synchronized (_introspectedClasses) + try (AutoLock ignored = _lock.lock()) { - //Synchronize on the set of already introspected classes. - //This ensures that only 1 thread can be introspecting, and that - //thread must have fully finished generating the products of - //introspection before another thread is allowed in. - //We remember the classes that we have introspected to avoid - //reprocessing the same class. + // Lock to ensure that only 1 thread can be introspecting, and that + // thread must have fully finished generating the products of + // the introspection before another thread is allowed in. + // We remember the classes that we have introspected to avoid + // reprocessing the same class. if (_introspectedClasses.add(clazz)) { for (IntrospectableAnnotationHandler handler : _handlers) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpChannel.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpChannel.java index ef62918a691..7f1519a3270 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpChannel.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpChannel.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.client; import org.eclipse.jetty.client.api.Result; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -26,6 +27,7 @@ public abstract class HttpChannel { private static final Logger LOG = LoggerFactory.getLogger(HttpChannel.class); + private final AutoLock _lock = new AutoLock(); private final HttpDestination _destination; private final TimeoutCompleteListener _totalTimeout; private HttpExchange _exchange; @@ -58,7 +60,7 @@ public abstract class HttpChannel { boolean result = false; boolean abort = true; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_exchange == null) { @@ -85,7 +87,7 @@ public abstract class HttpChannel public boolean disassociate(HttpExchange exchange) { boolean result = false; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { HttpExchange existing = _exchange; _exchange = null; @@ -103,10 +105,7 @@ public abstract class HttpChannel public HttpExchange getHttpExchange() { - synchronized (this) - { - return _exchange; - } + return _lock.runLocked(() -> _exchange); } protected abstract HttpSender getHttpSender(); 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 39fde1c7e08..f3c0e136d29 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 @@ -36,6 +36,7 @@ import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.util.HttpCookieStore; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,6 +44,7 @@ public abstract class HttpConnection implements IConnection { private static final Logger LOG = LoggerFactory.getLogger(HttpConnection.class); + private final AutoLock lock = new AutoLock(); private final HttpDestination destination; private int idleTimeoutGuard; private long idleTimeoutStamp; @@ -87,7 +89,7 @@ public abstract class HttpConnection implements IConnection // the request is associated to the channel and sent. // Use a counter to support multiplexed requests. boolean send; - synchronized (this) + try (AutoLock ignored = lock.lock()) { send = idleTimeoutGuard >= 0; if (send) @@ -111,7 +113,7 @@ public abstract class HttpConnection implements IConnection result = new SendFailure(new HttpRequestException("Could not associate request to connection", request), false); } - synchronized (this) + try (AutoLock ignored = lock.lock()) { --idleTimeoutGuard; idleTimeoutStamp = System.nanoTime(); @@ -250,7 +252,7 @@ public abstract class HttpConnection implements IConnection public boolean onIdleTimeout(long idleTimeout) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (idleTimeoutGuard == 0) { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java index 75a89dcefe4..01b81fcc2db 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java @@ -23,6 +23,7 @@ import java.util.List; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; import org.eclipse.jetty.client.api.Result; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -30,6 +31,7 @@ public class HttpExchange { private static final Logger LOG = LoggerFactory.getLogger(HttpExchange.class); + private final AutoLock lock = new AutoLock(); private final HttpDestination destination; private final HttpRequest request; private final List listeners; @@ -68,10 +70,7 @@ public class HttpExchange public Throwable getRequestFailure() { - synchronized (this) - { - return requestFailure; - } + return lock.runLocked(() -> requestFailure); } public List getResponseListeners() @@ -86,10 +85,7 @@ public class HttpExchange public Throwable getResponseFailure() { - synchronized (this) - { - return responseFailure; - } + return lock.runLocked(() -> responseFailure); } /** @@ -103,7 +99,7 @@ public class HttpExchange { boolean result = false; boolean abort = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { // Only associate if the exchange state is initial, // as the exchange could be already failed. @@ -127,7 +123,7 @@ public class HttpExchange void disassociate(HttpChannel channel) { boolean abort = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (_channel != channel || requestState != State.TERMINATED || responseState != State.TERMINATED) abort = true; @@ -140,18 +136,12 @@ public class HttpExchange private HttpChannel getHttpChannel() { - synchronized (this) - { - return _channel; - } + return lock.runLocked(() -> _channel); } public boolean requestComplete(Throwable failure) { - synchronized (this) - { - return completeRequest(failure); - } + return lock.runLocked(() -> completeRequest(failure)); } private boolean completeRequest(Throwable failure) @@ -167,10 +157,7 @@ public class HttpExchange public boolean responseComplete(Throwable failure) { - synchronized (this) - { - return completeResponse(failure); - } + return lock.runLocked(() -> completeResponse(failure)); } private boolean completeResponse(Throwable failure) @@ -187,7 +174,7 @@ public class HttpExchange public Result terminateRequest() { Result result = null; - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (requestState == State.COMPLETED) requestState = State.TERMINATED; @@ -204,7 +191,7 @@ public class HttpExchange public Result terminateResponse() { Result result = null; - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (responseState == State.COMPLETED) responseState = State.TERMINATED; @@ -224,7 +211,7 @@ public class HttpExchange // This will avoid that this exchange can be associated to a channel. boolean abortRequest; boolean abortResponse; - synchronized (this) + try (AutoLock ignored = lock.lock()) { abortRequest = completeRequest(failure); abortResponse = completeResponse(failure); @@ -283,7 +270,7 @@ public class HttpExchange public void resetResponse() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { responseState = State.PENDING; responseFailure = null; @@ -300,7 +287,7 @@ public class HttpExchange @Override public String toString() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { return String.format("%s@%x req=%s/%s@%h res=%s/%s@%h", HttpExchange.class.getSimpleName(), diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java index 59322a3f44e..6325c01864c 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpReceiver.java @@ -42,6 +42,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.MathUtils; import org.eclipse.jetty.util.component.Destroyable; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -74,6 +75,7 @@ public abstract class HttpReceiver { private static final Logger LOG = LoggerFactory.getLogger(HttpReceiver.class); + private final AutoLock lock = new AutoLock(); private final AtomicReference responseState = new AtomicReference<>(ResponseState.IDLE); private final HttpChannel channel; private ContentListeners contentListeners; @@ -98,7 +100,7 @@ public abstract class HttpReceiver throw new IllegalArgumentException("Invalid demand " + n); boolean resume = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { demand = MathUtils.cappedAdd(demand, n); if (stalled) @@ -126,15 +128,12 @@ public abstract class HttpReceiver private long demand(LongUnaryOperator operator) { - synchronized (this) - { - return demand = operator.applyAsLong(demand); - } + return lock.runLocked(() -> demand = operator.applyAsLong(demand)); } protected boolean hasDemandOrStall() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { stalled = demand <= 0; return !stalled; 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 3af05ee8e19..d2034866470 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 @@ -32,6 +32,7 @@ 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; @@ -40,6 +41,7 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C { 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; @@ -80,19 +82,13 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C @Override public int getMaxMultiplex() { - synchronized (this) - { - return maxMultiplex; - } + return lock.runLocked(() -> maxMultiplex); } @Override public void setMaxMultiplex(int maxMultiplex) { - synchronized (this) - { - this.maxMultiplex = maxMultiplex; - } + lock.runLocked(() -> this.maxMultiplex = maxMultiplex); } @Override @@ -103,7 +99,7 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C LOG.debug("Accepted {} {}", accepted, connection); if (accepted) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { Holder holder = new Holder(connection); activeConnections.put(connection, holder); @@ -117,20 +113,14 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C @Override public boolean isActive(Connection connection) { - synchronized (this) - { - return activeConnections.containsKey(connection); - } + return lock.runLocked(() -> activeConnections.containsKey(connection)); } @Override protected void onCreated(Connection connection) { - synchronized (this) - { - // Use "cold" connections as last. - idleConnections.offer(new Holder(connection)); - } + // Use "cold" connections as last. + lock.runLocked(() -> idleConnections.offer(new Holder(connection))); idle(connection, false); } @@ -138,7 +128,7 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C protected Connection activate() { Holder result = null; - synchronized (this) + try (AutoLock ignored = lock.lock()) { for (Holder holder : activeConnections.values()) { @@ -169,7 +159,7 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C boolean closed = isClosed(); boolean idle = false; Holder holder; - synchronized (this) + try (AutoLock ignored = lock.lock()) { holder = activeConnections.get(connection); if (holder != null) @@ -205,7 +195,7 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C { boolean activeRemoved = true; boolean idleRemoved = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { Holder holder = activeConnections.remove(connection); if (holder == null) @@ -236,7 +226,7 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C { super.close(); List connections; - synchronized (this) + try (AutoLock ignored = lock.lock()) { connections = idleConnections.stream().map(holder -> holder.connection).collect(Collectors.toList()); connections.addAll(activeConnections.keySet()); @@ -249,7 +239,7 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C { DumpableCollection active; DumpableCollection idle; - synchronized (this) + try (AutoLock ignored = lock.lock()) { active = new DumpableCollection("active", new ArrayList<>(activeConnections.values())); idle = new DumpableCollection("idle", new ArrayList<>(idleConnections)); @@ -261,7 +251,7 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C public boolean sweep() { List toSweep = new ArrayList<>(); - synchronized (this) + try (AutoLock ignored = lock.lock()) { activeConnections.values().stream() .map(holder -> holder.connection) @@ -289,7 +279,7 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C { int activeSize; int idleSize; - synchronized (this) + try (AutoLock ignored = lock.lock()) { activeSize = activeConnections.size(); idleSize = idleConnections.size(); 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 e13fba9390d..c45bad9115c 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 @@ -26,10 +26,12 @@ import org.eclipse.jetty.client.api.Connection; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.Dumpable; +import org.eclipse.jetty.util.thread.AutoLock; @ManagedObject public class RoundRobinConnectionPool extends AbstractConnectionPool implements ConnectionPool.Multiplexable { + private final AutoLock lock = new AutoLock(); private final List entries; private int maxMultiplex; private int index; @@ -53,19 +55,13 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements @Override public int getMaxMultiplex() { - synchronized (this) - { - return maxMultiplex; - } + return lock.runLocked(() -> maxMultiplex); } @Override public void setMaxMultiplex(int maxMultiplex) { - synchronized (this) - { - this.maxMultiplex = maxMultiplex; - } + lock.runLocked(() -> this.maxMultiplex = maxMultiplex); } /** @@ -86,7 +82,7 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements @Override protected void onCreated(Connection connection) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { for (Entry entry : entries) { @@ -104,7 +100,7 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements protected Connection activate() { Connection connection = null; - synchronized (this) + try (AutoLock ignored = lock.lock()) { int offset = 0; int capacity = getMaxConnectionCount(); @@ -139,7 +135,7 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements @Override public boolean isActive(Connection connection) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { for (Entry entry : entries) { @@ -155,7 +151,7 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements { boolean found = false; boolean idle = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { for (Entry entry : entries) { @@ -180,7 +176,7 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements public boolean remove(Connection connection) { boolean found = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { for (Entry entry : entries) { @@ -203,11 +199,7 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements @Override public void dump(Appendable out, String indent) throws IOException { - List connections; - synchronized (this) - { - connections = new ArrayList<>(entries); - } + List connections = lock.runLocked(() -> new ArrayList<>(entries)); Dumpable.dumpObjects(out, indent, out, connections); } @@ -216,7 +208,7 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements { int present = 0; int active = 0; - synchronized (this) + try (AutoLock ignored = lock.lock()) { for (Entry entry : entries) { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/util/AsyncRequestContent.java b/jetty-client/src/main/java/org/eclipse/jetty/client/util/AsyncRequestContent.java index 2e084786d0c..79ac545212e 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/util/AsyncRequestContent.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/util/AsyncRequestContent.java @@ -27,7 +27,6 @@ import java.util.ArrayDeque; import java.util.Deque; import java.util.List; import java.util.Objects; -import java.util.concurrent.locks.Condition; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -42,8 +41,7 @@ public class AsyncRequestContent implements Request.Content, Request.Content.Sub { private static final Logger LOG = LoggerFactory.getLogger(AsyncRequestContent.class); - private final AutoLock lock = new AutoLock(); - private final Condition flush = lock.newCondition(); + private final AutoLock.WithCondition lock = new AutoLock.WithCondition(); private final Deque chunks = new ArrayDeque<>(); private final String contentType; private long length = -1; @@ -118,7 +116,7 @@ public class AsyncRequestContent implements Request.Content, Request.Content.Sub public void fail(Throwable failure) { List toFail = List.of(); - try (AutoLock ignored = lock.lock()) + try (AutoLock.WithCondition l = lock.lock()) { if (this.failure == null) { @@ -128,7 +126,7 @@ public class AsyncRequestContent implements Request.Content, Request.Content.Sub .map(chunk -> chunk.callback) .collect(Collectors.toList()); chunks.clear(); - flush.signal(); + l.signal(); } } toFail.forEach(c -> c.failed(failure)); @@ -293,15 +291,15 @@ public class AsyncRequestContent implements Request.Content, Request.Content.Sub private void notifyFlush() { - try (AutoLock ignored = lock.lock()) + try (AutoLock.WithCondition l = lock.lock()) { - flush.signal(); + l.signal(); } } public void flush() throws IOException { - try (AutoLock ignored = lock.lock()) + try (AutoLock.WithCondition l = lock.lock()) { try { @@ -313,7 +311,7 @@ public class AsyncRequestContent implements Request.Content, Request.Content.Sub throw new IOException(failure); if (chunks.isEmpty()) return; - flush.await(); + l.await(); } } catch (InterruptedException x) @@ -327,7 +325,7 @@ public class AsyncRequestContent implements Request.Content, Request.Content.Sub public void close() { boolean produce = false; - try (AutoLock ignored = lock.lock()) + try (AutoLock.WithCondition l = lock.lock()) { if (closed) return; @@ -340,7 +338,7 @@ public class AsyncRequestContent implements Request.Content, Request.Content.Sub produce = true; } } - flush.signal(); + l.signal(); } if (produce) produce(); diff --git a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java index 3793d99f7b6..b8565032e87 100644 --- a/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java +++ b/jetty-deploy/src/main/java/org/eclipse/jetty/deploy/DeploymentManager.java @@ -562,7 +562,7 @@ public class DeploymentManager extends ContainerLifeCycle requestAppGoal(appentry, nodeName); } - private synchronized void addOnStartupError(Throwable cause) + private void addOnStartupError(Throwable cause) { if (onStartupErrors == null) { 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 16badda409a..9a5d950c28a 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 @@ -54,6 +54,7 @@ import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -61,6 +62,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne { private static final Logger LOG = LoggerFactory.getLogger(HttpConnectionOverFCGI.class); + private final AutoLock lock = new AutoLock(); private final LinkedList requests = new LinkedList<>(); private final Map activeChannels = new ConcurrentHashMap<>(); private final Queue idleChannels = new ConcurrentLinkedQueue<>(); @@ -307,7 +309,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne private int acquireRequest() { - synchronized (requests) + try (AutoLock ignored = lock.lock()) { int last = requests.getLast(); int request = last + 1; @@ -318,10 +320,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne private void releaseRequest(int request) { - synchronized (requests) - { - requests.removeFirstOccurrence(request); - } + lock.runLocked(() -> requests.removeFirstOccurrence(request)); } protected HttpChannelOverFCGI acquireHttpChannel(int id, Request request) diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/generator/Flusher.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/generator/Flusher.java index 0b31c501a4a..66283bc28e1 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/generator/Flusher.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/generator/Flusher.java @@ -24,6 +24,7 @@ import java.util.Queue; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.util.IteratingCallback; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,6 +32,7 @@ public class Flusher { private static final Logger LOG = LoggerFactory.getLogger(Flusher.class); + private final AutoLock lock = new AutoLock(); private final Queue queue = new ArrayDeque<>(); private final IteratingCallback flushCallback = new FlushCallback(); private final EndPoint endPoint; @@ -51,18 +53,12 @@ public class Flusher private void offer(Generator.Result result) { - synchronized (this) - { - queue.offer(result); - } + lock.runLocked(() -> queue.offer(result)); } private Generator.Result poll() { - synchronized (this) - { - return queue.poll(); - } + return lock.runLocked(queue::poll); } public void shutdown() diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java index f377b566cfb..2eb61902cd1 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/MimeTypes.java @@ -291,7 +291,7 @@ public class MimeTypes { } - public synchronized Map getMimeMap() + public Map getMimeMap() { return _mimeMap; } diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/RawHTTP2ProxyTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/RawHTTP2ProxyTest.java index 3ebfc00e557..58a7484189c 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/RawHTTP2ProxyTest.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/RawHTTP2ProxyTest.java @@ -53,6 +53,7 @@ import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FuturePromise; import org.eclipse.jetty.util.IteratingCallback; import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -316,7 +317,7 @@ public class RawHTTP2ProxyTest private static class ClientToProxyToServer extends IteratingCallback implements Stream.Listener { - private final Object lock = this; + private final AutoLock lock = new AutoLock(); private final Map> frames = new HashMap<>(); private final Map streams = new HashMap<>(); private final ServerToProxyToClient serverToProxyToClient = new ServerToProxyToClient(); @@ -339,7 +340,7 @@ public class RawHTTP2ProxyTest if (LOGGER.isDebugEnabled()) LOGGER.debug("CPS queueing {} for {} on {}", frame, stream, stream.getSession()); boolean connected; - synchronized (lock) + try (AutoLock ignored = lock.lock()) { Deque deque = frames.computeIfAbsent(stream, s -> new ArrayDeque<>()); deque.offer(new FrameInfo(frame, callback)); @@ -363,10 +364,7 @@ public class RawHTTP2ProxyTest { if (LOGGER.isDebugEnabled()) LOGGER.debug("CPS connected to {} with {}", address, result); - synchronized (lock) - { - proxyToServerSession = result; - } + lock.runLocked(() -> proxyToServerSession = result); iterate(); } @@ -385,7 +383,7 @@ public class RawHTTP2ProxyTest { Stream proxyToServerStream = null; Session proxyToServerSession = null; - synchronized (lock) + try (AutoLock ignored = lock.lock()) { for (Map.Entry> entry : frames.entrySet()) { @@ -415,12 +413,9 @@ public class RawHTTP2ProxyTest @Override public void succeeded(Stream result) { - synchronized (lock) - { - if (LOGGER.isDebugEnabled()) - LOGGER.debug("CPS created {}", result); - streams.put(clientToProxyStream, result); - } + if (LOGGER.isDebugEnabled()) + LOGGER.debug("CPS created {}", result); + lock.runLocked(() -> streams.put(clientToProxyStream, result)); serverToProxyToClient.link(result, clientToProxyStream); ClientToProxyToServer.this.succeeded(); } @@ -549,17 +544,17 @@ public class RawHTTP2ProxyTest private static class ServerToProxyToClient extends IteratingCallback implements Stream.Listener { - private final Object lock = this; + private final AutoLock lock = new AutoLock(); private final Map> frames = new HashMap<>(); private final Map streams = new HashMap<>(); private FrameInfo frameInfo; private Stream serverToProxyStream; @Override - protected Action process() throws Throwable + protected Action process() { Stream proxyToClientStream = null; - synchronized (lock) + try (AutoLock ignored = lock.lock()) { for (Map.Entry> entry : frames.entrySet()) { @@ -630,7 +625,7 @@ public class RawHTTP2ProxyTest { if (LOGGER.isDebugEnabled()) LOGGER.debug("SPC queueing {} for {} on {}", frame, stream, stream.getSession()); - synchronized (lock) + try (AutoLock ignored = lock.lock()) { Deque deque = frames.computeIfAbsent(stream, s -> new ArrayDeque<>()); deque.offer(new FrameInfo(frame, callback)); @@ -682,10 +677,7 @@ public class RawHTTP2ProxyTest private void link(Stream proxyToServerStream, Stream clientToProxyStream) { - synchronized (lock) - { - streams.put(proxyToServerStream, clientToProxyStream); - } + lock.runLocked(() -> streams.put(proxyToServerStream, clientToProxyStream)); iterate(); } } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java index 206b869e58e..b0cdbbf5a50 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java @@ -36,6 +36,7 @@ import org.eclipse.jetty.io.WriteFlusher; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.component.LifeCycle; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ExecutionStrategy; import org.eclipse.jetty.util.thread.TryExecutor; import org.eclipse.jetty.util.thread.strategy.EatWhatYouKill; @@ -49,6 +50,7 @@ public class HTTP2Connection extends AbstractConnection implements WriteFlusher. // TODO remove this once we are sure EWYK is OK for http2 private static final boolean PEC_MODE = Boolean.getBoolean("org.eclipse.jetty.http2.PEC_MODE"); + private final AutoLock lock = new AutoLock(); private final Queue tasks = new ArrayDeque<>(); private final HTTP2Producer producer = new HTTP2Producer(); private final AtomicLong bytesIn = new AtomicLong(); @@ -190,10 +192,7 @@ public class HTTP2Connection extends AbstractConnection implements WriteFlusher. private void offerTask(Runnable task) { - synchronized (this) - { - tasks.offer(task); - } + lock.runLocked(() -> tasks.offer(task)); } protected void produce() @@ -220,10 +219,7 @@ public class HTTP2Connection extends AbstractConnection implements WriteFlusher. private Runnable pollTask() { - synchronized (this) - { - return tasks.poll(); - } + return lock.runLocked(tasks::poll); } @Override diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Flusher.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Flusher.java index 93ffc90c3ac..b87e5f711e0 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Flusher.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Flusher.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IteratingCallback; import org.eclipse.jetty.util.component.Dumpable; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -46,6 +47,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable private static final Logger LOG = LoggerFactory.getLogger(HTTP2Flusher.class); private static final ByteBuffer[] EMPTY_BYTE_BUFFERS = new ByteBuffer[0]; + private final AutoLock lock = new AutoLock(); private final Queue windows = new ArrayDeque<>(); private final Deque entries = new ArrayDeque<>(); private final Queue pendingEntries = new ArrayDeque<>(); @@ -64,7 +66,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable public void window(IStream stream, WindowUpdateFrame frame) { Throwable closed; - synchronized (this) + try (AutoLock ignored = lock.lock()) { closed = terminated; if (closed == null) @@ -78,7 +80,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable public boolean prepend(Entry entry) { Throwable closed; - synchronized (this) + try (AutoLock ignored = lock.lock()) { closed = terminated; if (closed == null) @@ -97,7 +99,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable public boolean append(Entry entry) { Throwable closed; - synchronized (this) + try (AutoLock ignored = lock.lock()) { closed = terminated; if (closed == null) @@ -115,18 +117,12 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable private int getWindowQueueSize() { - synchronized (this) - { - return windows.size(); - } + return lock.runLocked(windows::size); } public int getFrameQueueSize() { - synchronized (this) - { - return entries.size(); - } + return lock.runLocked(entries::size); } @Override @@ -135,7 +131,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable if (LOG.isDebugEnabled()) LOG.debug("Flushing {}", session); - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (terminated != null) throw terminated; @@ -323,7 +319,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable Throwable closed; Set allEntries; - synchronized (this) + try (AutoLock ignored = lock.lock()) { closed = terminated; terminated = x; @@ -352,7 +348,7 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable void terminate(Throwable cause) { Throwable closed; - synchronized (this) + try (AutoLock ignored = lock.lock()) { closed = terminated; terminated = cause; diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 107cb15be93..597938b1573 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -70,6 +70,7 @@ import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.component.DumpableCollection; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Scheduler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -1748,6 +1749,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio */ private class StreamCreator { + private final AutoLock lock = new AutoLock(); private final Queue slots = new ArrayDeque<>(); private Thread flushing; @@ -1825,7 +1827,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio { if (streamId <= 0) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { streamId = localStreamIds.getAndAdd(2); slots.offer(slot); @@ -1833,20 +1835,14 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio } else { - synchronized (this) - { - slots.offer(slot); - } + lock.runLocked(() -> slots.offer(slot)); } return streamId; } private void releaseSlotFlushAndFail(Slot slot, Promise promise, Throwable x) { - synchronized (this) - { - slots.remove(slot); - } + lock.runLocked(() -> slots.remove(slot)); flush(); promise.failed(x); } @@ -1870,7 +1866,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio while (true) { ControlEntry entry; - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (flushing == null) flushing = thread; diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java index e13c7352e49..ac24709793a 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java @@ -143,7 +143,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa @Override public void reset(ResetFrame frame, Callback callback) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (isReset()) return; @@ -156,7 +156,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa private boolean startWrite(Callback callback) { Throwable failure; - synchronized (this) + try (AutoLock ignored = lock.lock()) { failure = this.failure; if (failure == null && sendCallback == null) @@ -192,27 +192,18 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa @Override public boolean isReset() { - synchronized (this) - { - return localReset || remoteReset; - } + return lock.runLocked(() -> localReset || remoteReset); } private boolean isFailed() { - synchronized (this) - { - return failure != null; - } + return lock.runLocked(() -> failure != null); } @Override public boolean isResetOrFailed() { - synchronized (this) - { - return isReset() || isFailed(); - } + return lock.runLocked(() -> isReset() || isFailed()); } @Override @@ -391,7 +382,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa boolean initial; boolean proceed = false; DataEntry entry = new DataEntry(frame, callback); - try (AutoLock l = lock.lock()) + try (AutoLock ignored = lock.lock()) { dataQueue.offer(entry); initial = dataInitial; @@ -412,7 +403,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa if (LOG.isDebugEnabled()) LOG.debug("Starting data processing of {} for {}", frame, this); notifyBeforeData(this); - try (AutoLock l = lock.lock()) + try (AutoLock ignored = lock.lock()) { dataProcess = proceed = dataDemand > 0; } @@ -430,7 +421,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa throw new IllegalArgumentException("Invalid demand " + n); long demand; boolean proceed = false; - try (AutoLock l = lock.lock()) + try (AutoLock ignored = lock.lock()) { demand = dataDemand = MathUtils.cappedAdd(dataDemand, n); if (!dataProcess) @@ -447,7 +438,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa while (true) { DataEntry dataEntry; - try (AutoLock l = lock.lock()) + try (AutoLock ignored = lock.lock()) { if (dataQueue.isEmpty() || dataDemand == 0) { @@ -468,15 +459,12 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa private long demand() { - try (AutoLock l = lock.lock()) - { - return dataDemand; - } + return lock.runLocked(() -> dataDemand); } private void onReset(ResetFrame frame, Callback callback) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { remoteReset = true; failure = new EofException("reset"); @@ -501,10 +489,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa private void onFailure(FailureFrame frame, Callback callback) { - synchronized (this) - { - failure = frame.getFailure(); - } + lock.runLocked(() -> failure = frame.getFailure()); close(); session.removeStream(this); notifyFailure(this, frame, callback); @@ -689,7 +674,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa private Callback endWrite() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { Callback callback = sendCallback; sendCallback = null; diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2StreamEndPoint.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2StreamEndPoint.java index 5c3e5bbc19e..e60525ad3e0 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2StreamEndPoint.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2StreamEndPoint.java @@ -35,6 +35,7 @@ import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Invocable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,6 +44,7 @@ public abstract class HTTP2StreamEndPoint implements EndPoint { private static final Logger LOG = LoggerFactory.getLogger(HTTP2StreamEndPoint.class); + private final AutoLock lock = new AutoLock(); private final Deque dataQueue = new ArrayDeque<>(); private final AtomicReference writeState = new AtomicReference<>(WriteState.IDLE); private final AtomicReference readCallback = new AtomicReference<>(); @@ -171,11 +173,7 @@ public abstract class HTTP2StreamEndPoint implements EndPoint @Override public int fill(ByteBuffer sink) throws IOException { - Entry entry; - synchronized (this) - { - entry = dataQueue.poll(); - } + Entry entry = lock.runLocked(dataQueue::poll); if (LOG.isDebugEnabled()) LOG.debug("filled {} on {}", entry, this); @@ -206,10 +204,7 @@ public abstract class HTTP2StreamEndPoint implements EndPoint if (source.hasRemaining()) { - synchronized (this) - { - dataQueue.offerFirst(entry); - } + lock.runLocked(() -> dataQueue.offerFirst(entry)); } else { @@ -548,19 +543,13 @@ public abstract class HTTP2StreamEndPoint implements EndPoint private void offer(ByteBuffer buffer, Callback callback, Throwable failure) { - synchronized (this) - { - dataQueue.offer(new Entry(buffer, callback, failure)); - } + Entry entry = new Entry(buffer, callback, failure); + lock.runLocked(() -> dataQueue.offer(entry)); } protected void process() { - boolean empty; - synchronized (this) - { - empty = dataQueue.isEmpty(); - } + boolean empty = lock.runLocked(dataQueue::isEmpty); if (!empty) { Callback callback = readCallback.getAndSet(null); diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java index e2cfc1e8650..24ac5ef4938 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java @@ -50,6 +50,7 @@ import org.eclipse.jetty.http2.frames.ResetFrame; import org.eclipse.jetty.io.EndPoint; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -246,6 +247,7 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements HTTP2Channel. private class ContentNotifier { + private final AutoLock lock = new AutoLock(); private final Queue queue = new ArrayDeque<>(); private final HttpReceiverOverHTTP2 receiver; private DataInfo dataInfo; @@ -269,10 +271,7 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements HTTP2Channel. private void enqueue(DataInfo dataInfo) { - synchronized (this) - { - queue.offer(dataInfo); - } + lock.runLocked(() -> queue.offer(dataInfo)); } private void process(boolean resume) @@ -285,7 +284,7 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements HTTP2Channel. return; // Process only if there is demand. - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (!resume && demand() <= 0) { @@ -309,7 +308,7 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements HTTP2Channel. } } - synchronized (this) + try (AutoLock ignored = lock.lock()) { dataInfo = queue.poll(); if (LOG.isDebugEnabled()) @@ -347,7 +346,7 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements HTTP2Channel. private boolean active(boolean resume) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (active) { @@ -380,7 +379,7 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements HTTP2Channel. */ private boolean stall() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (resume) { @@ -400,7 +399,7 @@ public class HttpReceiverOverHTTP2 extends HttpReceiver implements HTTP2Channel. private void reset() { dataInfo = null; - synchronized (this) + try (AutoLock ignored = lock.lock()) { queue.clear(); active = false; diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnection.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnection.java index efce84ed39e..7143d43c521 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnection.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnection.java @@ -58,6 +58,7 @@ import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.TypeUtil; +import org.eclipse.jetty.util.thread.AutoLock; public class HTTP2ServerConnection extends HTTP2Connection { @@ -84,7 +85,8 @@ public class HTTP2ServerConnection extends HTTP2Connection return false; } } - + + private final AutoLock lock = new AutoLock(); private final Queue channels = new ArrayDeque<>(); private final List upgradeFrames = new ArrayList<>(); private final AtomicLong totalRequests = new AtomicLong(); @@ -286,27 +288,15 @@ public class HTTP2ServerConnection extends HTTP2Connection private void offerHttpChannel(HttpChannelOverHTTP2 channel) { if (isRecycleHttpChannels()) - { - synchronized (this) - { - channels.offer(channel); - } - } + lock.runLocked(() -> channels.offer(channel)); } private HttpChannelOverHTTP2 pollHttpChannel() { if (isRecycleHttpChannels()) - { - synchronized (this) - { - return channels.poll(); - } - } + return lock.runLocked(channels::poll); else - { return null; - } } public boolean upgrade(Request request, HttpFields.Mutable responseFields) diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java index e08993de746..057bac13eb8 100644 --- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java +++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java @@ -44,6 +44,7 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.Promise; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -413,6 +414,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport */ private class TransportCallback implements Callback { + private final AutoLock _lock = new AutoLock(); private State _state = State.IDLE; private Callback _callback; private boolean _commit; @@ -420,7 +422,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport private void reset(Throwable failure) { - assert Thread.holdsLock(this); + assert _lock.isHeldByCurrentThread(); _state = failure != null ? State.FAILED : State.IDLE; _callback = null; _commit = false; @@ -443,7 +445,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport private Throwable sending(Callback callback, boolean commit) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { switch (_state) { @@ -471,7 +473,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport { Callback callback; boolean commit; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_state != State.SENDING) { @@ -495,7 +497,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport { Callback callback; boolean commit; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_state != State.SENDING) { @@ -517,7 +519,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport private boolean idleTimeout(Throwable failure) { Callback callback = null; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { // Ignore idle timeouts if not writing, // as the application may be suspended. @@ -541,11 +543,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport @Override public InvocationType getInvocationType() { - Callback callback; - synchronized (this) - { - callback = _callback; - } + Callback callback = _lock.runLocked(() -> _callback); return callback != null ? callback.getInvocationType() : Callback.super.getInvocationType(); } } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java index bddd34d0bfc..3975fcccd08 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ByteArrayEndPoint.java @@ -31,7 +31,6 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayDeque; import java.util.Queue; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Condition; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.thread.AutoLock; @@ -69,8 +68,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint private static final ByteBuffer EOF = BufferUtil.allocate(0); private final Runnable _runFillable = () -> getFillInterest().fillable(); - private final AutoLock _lock = new AutoLock(); - private final Condition _hasOutput = _lock.newCondition(); + private final AutoLock.WithCondition _lock = new AutoLock.WithCondition(); private final Queue _inQ = new ArrayDeque<>(); private ByteBuffer _out; private boolean _growOutput; @@ -127,9 +125,9 @@ public class ByteArrayEndPoint extends AbstractEndPoint public void doShutdownOutput() { super.doShutdownOutput(); - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { - _hasOutput.signalAll(); + lock.signalAll(); } } @@ -137,9 +135,9 @@ public class ByteArrayEndPoint extends AbstractEndPoint public void doClose() { super.doClose(); - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { - _hasOutput.signalAll(); + lock.signalAll(); } } @@ -303,11 +301,11 @@ public class ByteArrayEndPoint extends AbstractEndPoint { ByteBuffer b; - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { while (BufferUtil.isEmpty(_out) && !isOutputShutdown()) { - if (!_hasOutput.await(time, unit)) + if (!lock.await(time, unit)) return null; } b = _out; @@ -401,7 +399,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint public boolean flush(ByteBuffer... buffers) throws IOException { boolean flushed = true; - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { if (!isOpen()) throw new IOException("CLOSED"); @@ -438,7 +436,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint if (!idle) { notIdle(); - _hasOutput.signalAll(); + lock.signalAll(); } } return flushed; @@ -447,11 +445,11 @@ public class ByteArrayEndPoint extends AbstractEndPoint @Override public void reset() { - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { _inQ.clear(); - _hasOutput.signalAll(); BufferUtil.clear(_out); + lock.signalAll(); } super.reset(); } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java index 47651f816a0..4f856097ec9 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java @@ -48,6 +48,7 @@ import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.DumpableCollection; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ExecutionStrategy; import org.eclipse.jetty.util.thread.Scheduler; import org.eclipse.jetty.util.thread.strategy.EatWhatYouKill; @@ -79,6 +80,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable } } + private final AutoLock _lock = new AutoLock(); private final AtomicBoolean _started = new AtomicBoolean(false); private boolean _selecting; private final SelectorManager _selectorManager; @@ -252,7 +254,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable LOG.debug("Queued change lazy={} {} on {}", lazy, update, this); Selector selector = null; - synchronized (ManagedSelector.this) + try (AutoLock ignored = _lock.lock()) { _updates.offer(update); @@ -278,7 +280,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable LOG.debug("Wakeup {}", this); Selector selector = null; - synchronized (ManagedSelector.this) + try (AutoLock ignored = _lock.lock()) { if (_selecting) { @@ -382,10 +384,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable private int getActionSize() { - synchronized (ManagedSelector.this) - { - return _updates.size(); - } + return _lock.runLocked(() -> _updates.size()); } static int safeReadyOps(SelectionKey selectionKey) @@ -424,7 +423,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable { DumpKeys dump = new DumpKeys(); String updatesAt = DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(ZonedDateTime.now()); - synchronized (ManagedSelector.this) + try (AutoLock ignored = _lock.lock()) { updates = new ArrayList<>(_updates); _updates.addFirst(dump); @@ -514,7 +513,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable private void processUpdates() { - synchronized (ManagedSelector.this) + try (AutoLock ignored = _lock.lock()) { Deque updates = _updates; _updates = _updateable; @@ -543,7 +542,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable Selector selector; int updates; - synchronized (ManagedSelector.this) + try (AutoLock ignored = _lock.lock()) { updates = _updates.size(); _selecting = updates == 0; @@ -579,7 +578,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable LOG.debug("Selector {} woken up from select, {}/{}/{} selected", selector, selected, selector.selectedKeys().size(), selector.keys().size()); int updates; - synchronized (ManagedSelector.this) + try (AutoLock ignored = _lock.lock()) { // finished selecting _selecting = false; diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SocketChannelEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SocketChannelEndPoint.java index e662c104b58..3d4ce44c665 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SocketChannelEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SocketChannelEndPoint.java @@ -29,6 +29,7 @@ import java.nio.channels.Selector; import java.nio.channels.SocketChannel; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Invocable; import org.eclipse.jetty.util.thread.Scheduler; import org.slf4j.Logger; @@ -42,6 +43,7 @@ public class SocketChannelEndPoint extends AbstractEndPoint implements ManagedSe { private static final Logger LOG = LoggerFactory.getLogger(SocketChannelEndPoint.class); + private final AutoLock _lock = new AutoLock(); private final SocketChannel _channel; private final ManagedSelector _selector; private SelectionKey _key; @@ -317,7 +319,7 @@ public class SocketChannelEndPoint extends AbstractEndPoint implements ManagedSe int readyOps = _key.readyOps(); int oldInterestOps; int newInterestOps; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _updatePending = true; // Remove the readyOps, that here can only be OP_READ or OP_WRITE (or both). @@ -361,7 +363,7 @@ public class SocketChannelEndPoint extends AbstractEndPoint implements ManagedSe { int oldInterestOps; int newInterestOps; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _updatePending = false; oldInterestOps = _currentInterestOps; @@ -403,7 +405,7 @@ public class SocketChannelEndPoint extends AbstractEndPoint implements ManagedSe int oldInterestOps; int newInterestOps; boolean pending; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { pending = _updatePending; oldInterestOps = _desiredInterestOps; diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/WriterOutputStream.java b/jetty-io/src/main/java/org/eclipse/jetty/io/WriterOutputStream.java index 589c34d7c62..a78b0e863f0 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/WriterOutputStream.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/WriterOutputStream.java @@ -33,7 +33,6 @@ public class WriterOutputStream extends OutputStream { protected final Writer _writer; protected final Charset _encoding; - private final byte[] _buf = new byte[1]; public WriterOutputStream(Writer writer, String encoding) { @@ -82,11 +81,9 @@ public class WriterOutputStream extends OutputStream } @Override - public synchronized void write(int b) + public void write(int b) throws IOException { - _buf[0] = (byte)b; - write(_buf); + write(new byte[]{(byte)b}); } } - diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 25caea62ad8..f771c08f6e8 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -43,6 +43,7 @@ import org.eclipse.jetty.io.WriteFlusher; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Invocable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -104,6 +105,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr WAIT_FOR_FILL // Waiting for a fill to happen } + private final AutoLock _lock = new AutoLock(); private final List handshakeListeners = new ArrayList<>(); private final ByteBufferPool _bufferPool; private final SSLEngine _sslEngine; @@ -435,7 +437,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr private void releaseEncryptedOutputBuffer() { - if (!Thread.holdsLock(_decryptedEndPoint)) + if (!_lock.isHeldByCurrentThread()) throw new IllegalStateException(); if (_encryptedOutput != null && !_encryptedOutput.hasRemaining()) { @@ -514,7 +516,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr { // If we are handshaking, then wake up any waiting write as well as it may have been blocked on the read boolean waitingForFill; - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug("onFillable {}", SslConnection.this); @@ -527,10 +529,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr if (waitingForFill) { - synchronized (_decryptedEndPoint) - { - waitingForFill = _flushState == FlushState.WAIT_FOR_FILL; - } + waitingForFill = _lock.runLocked(() -> _flushState == FlushState.WAIT_FOR_FILL); if (waitingForFill) fill(BufferUtil.EMPTY_BUFFER); } @@ -545,7 +544,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr { // If we are handshaking, then wake up any waiting write as well as it may have been blocked on the read boolean fail = false; - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug("onFillableFail {}", SslConnection.this, failure); @@ -594,7 +593,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr { try { - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug(">fill {}", SslConnection.this); @@ -814,7 +813,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr boolean fillable; ByteBuffer write = null; boolean interest = false; - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug(">needFillInterest s={}/{} uf={} ei={} di={} {}", @@ -958,7 +957,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr { try { - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (LOG.isDebugEnabled()) { @@ -1148,7 +1147,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr { boolean fillInterest = false; ByteBuffer write = null; - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug(">onIncompleteFlush {} {}", SslConnection.this, BufferUtil.toDetailString(_encryptedOutput)); @@ -1242,7 +1241,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr { boolean close; boolean flush = false; - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { boolean ishut = endPoint.isInputShutdown(); boolean oshut = endPoint.isOutputShutdown(); @@ -1268,7 +1267,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr // If we still can't flush, but we are not closing the endpoint, // let's just flush the encrypted output in the background. ByteBuffer write = null; - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (BufferUtil.hasContent(_encryptedOutput)) { @@ -1280,7 +1279,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr { endPoint.write(Callback.from(() -> { - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { _flushState = FlushState.IDLE; releaseEncryptedOutputBuffer(); @@ -1455,7 +1454,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr private Throwable handleException(Throwable x, String context) { - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (_failure == null) { @@ -1497,7 +1496,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr { boolean fillable; boolean interested; - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug("IncompleteWriteCB succeeded {}", SslConnection.this); @@ -1522,7 +1521,7 @@ public class SslConnection extends AbstractConnection implements Connection.Upgr public void failed(final Throwable x) { boolean failFillInterest; - synchronized (_decryptedEndPoint) + try (AutoLock ignored = _lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug("IncompleteWriteCB failed {}", SslConnection.this, x); diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java index 3ad5bdd737e..1087c4b975e 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java @@ -214,7 +214,7 @@ public class SslConnectionTest } @Override - public synchronized void onFillable() + public void onFillable() { EndPoint endp = getEndPoint(); try diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/UserInfo.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/UserInfo.java index 5d47d117d70..3568b735a7a 100644 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/UserInfo.java +++ b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/UserInfo.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.List; import org.eclipse.jetty.util.security.Credential; +import org.eclipse.jetty.util.thread.AutoLock; /** * UserInfo @@ -34,10 +35,10 @@ import org.eclipse.jetty.util.security.Credential; */ public class UserInfo { - - private String _userName; - private Credential _credential; - protected List _roleNames = new ArrayList<>(); + private final AutoLock _lock = new AutoLock(); + protected final List _roleNames = new ArrayList<>(); + private final String _userName; + private final Credential _credential; protected boolean _rolesLoaded = false; /** @@ -80,7 +81,7 @@ public class UserInfo public void fetchRoles() throws Exception { - synchronized (_roleNames) + try (AutoLock ignored = _lock.lock()) { if (!_rolesLoaded) { diff --git a/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java b/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java index 96a9a1f2ba0..6460b964636 100644 --- a/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java +++ b/jetty-jndi/src/main/java/org/eclipse/jetty/jndi/ContextFactory.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.jndi; import java.io.IOException; -import java.util.Collections; import java.util.Hashtable; import java.util.Map; import java.util.WeakHashMap; @@ -32,6 +31,7 @@ import javax.naming.spi.ObjectFactory; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.component.Dumpable; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,12 +41,12 @@ import org.slf4j.LoggerFactory; * This is an object factory that produces a jndi naming * context based on a classloader. *

- * It is used for the java:comp context. + * It is used for the {@code java:comp} context. *

- * This object factory is bound at java:comp. When a + * This object factory is bound at {@code java:comp}. When a * lookup arrives for java:comp, this object factory * is invoked and will return a context specific to - * the caller's environment (so producing the java:comp/env + * the caller's environment (so producing the {@code java:comp/env} * specific to a webapp). *

* The context selected is based on classloaders. First @@ -63,19 +63,21 @@ public class ContextFactory implements ObjectFactory /** * Map of classloaders to contexts. */ - private static final Map __contextMap = Collections.synchronizedMap(new WeakHashMap<>()); + private static final Map __contextMap = new WeakHashMap<>(); /** * Threadlocal for injecting a context to use * instead of looking up the map. */ - private static final ThreadLocal __threadContext = new ThreadLocal(); + private static final ThreadLocal __threadContext = new ThreadLocal<>(); /** * Threadlocal for setting a classloader which must be used * when finding the comp context. */ - private static final ThreadLocal __threadClassLoader = new ThreadLocal(); + private static final ThreadLocal __threadClassLoader = new ThreadLocal<>(); + + private static final AutoLock __lock = new AutoLock(); /** * Find or create a context which pertains to a classloader. @@ -90,8 +92,6 @@ public class ContextFactory implements ObjectFactory *

* If there is no current jetty Context, or it has no associated classloader, we * return null. - * - * @see javax.naming.spi.ObjectFactory#getObjectInstance(java.lang.Object, javax.naming.Name, javax.naming.Context, java.util.Hashtable) */ @Override public Object getObjectInstance(Object obj, @@ -101,7 +101,7 @@ public class ContextFactory implements ObjectFactory throws Exception { //First, see if we have had a context injected into us to use. - Context ctx = (Context)__threadContext.get(); + Context ctx = __threadContext.get(); if (ctx != null) { if (LOG.isDebugEnabled()) @@ -111,12 +111,12 @@ public class ContextFactory implements ObjectFactory //See if there is a classloader to use for finding the comp context //Don't use its parent hierarchy if set. - ClassLoader loader = (ClassLoader)__threadClassLoader.get(); + ClassLoader loader = __threadClassLoader.get(); if (loader != null) { if (LOG.isDebugEnabled()) LOG.debug("Using threadlocal classloader"); - synchronized (__contextMap) + try (AutoLock ignored = __lock.lock()) { ctx = getContextForClassLoader(loader); if (ctx == null) @@ -137,12 +137,12 @@ public class ContextFactory implements ObjectFactory { if (LOG.isDebugEnabled()) LOG.debug("Trying thread context classloader"); - synchronized (__contextMap) + try (AutoLock ignored = __lock.lock()) { while (ctx == null && loader != null) { ctx = getContextForClassLoader(loader); - if (ctx == null && loader != null) + if (ctx == null) loader = loader.getParent(); } @@ -161,12 +161,12 @@ public class ContextFactory implements ObjectFactory //classloader associated with the current context if (ContextHandler.getCurrentContext() != null) { - if (LOG.isDebugEnabled() && loader != null) + if (LOG.isDebugEnabled()) LOG.debug("Trying classloader of current org.eclipse.jetty.server.handler.ContextHandler"); - synchronized (__contextMap) + try (AutoLock ignored = __lock.lock()) { loader = ContextHandler.getCurrentContext().getContextHandler().getClassLoader(); - ctx = (Context)__contextMap.get(loader); + ctx = __contextMap.get(loader); if (ctx == null && loader != null) { @@ -221,7 +221,7 @@ public class ContextFactory implements ObjectFactory if (loader == null) return null; - return (Context)__contextMap.get(loader); + return __lock.runLocked(() -> __contextMap.get(loader)); } /** @@ -233,7 +233,7 @@ public class ContextFactory implements ObjectFactory */ public static Context associateContext(final Context ctx) { - Context previous = (Context)__threadContext.get(); + Context previous = __threadContext.get(); __threadContext.set(ctx); return previous; } @@ -245,7 +245,7 @@ public class ContextFactory implements ObjectFactory public static ClassLoader associateClassLoader(final ClassLoader loader) { - ClassLoader prev = (ClassLoader)__threadClassLoader.get(); + ClassLoader prev = __threadClassLoader.get(); __threadClassLoader.set(loader); return prev; } @@ -257,7 +257,7 @@ public class ContextFactory implements ObjectFactory public static void dump(Appendable out, String indent) throws IOException { - synchronized (__contextMap) + try (AutoLock ignored = __lock.lock()) { Dumpable.dumpObjects(out, indent, String.format("o.e.j.jndi.ContextFactory@%x", __contextMap.hashCode()), __contextMap); } diff --git a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/utils/OSGiClassLoader.java b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/utils/OSGiClassLoader.java index 256e4e5aa4f..67d2300ead3 100644 --- a/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/utils/OSGiClassLoader.java +++ b/jetty-osgi/jetty-osgi-boot/src/main/java/org/eclipse/jetty/osgi/boot/utils/OSGiClassLoader.java @@ -96,37 +96,40 @@ public class OSGiClassLoader extends URLClassLoader } @Override - protected synchronized Class loadClass(String name, boolean resolve) throws ClassNotFoundException + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { - Class c = findLoadedClass(name); - ClassNotFoundException ex = null; - boolean triedParent = false; - - if (c == null) + synchronized (getClassLoadingLock(name)) { - try + Class c = findLoadedClass(name); + ClassNotFoundException ex = null; + boolean triedParent = false; + + if (c == null) { - c = this.findClass(name); - } - catch (ClassNotFoundException e) - { - ex = e; + try + { + c = this.findClass(name); + } + catch (ClassNotFoundException e) + { + ex = e; + } } + + if (c == null && _parent != null && !triedParent) + c = _parent.loadClass(name); + + if (c == null) + throw ex; + + if (resolve) + resolveClass(c); + + if (LOG.isDebugEnabled()) + LOG.debug("loaded " + c + " from " + c.getClassLoader()); + + return c; } - - if (c == null && _parent != null && !triedParent) - c = _parent.loadClass(name); - - if (c == null) - throw ex; - - if (resolve) - resolveClass(c); - - if (LOG.isDebugEnabled()) - LOG.debug("loaded " + c + " from " + c.getClassLoader()); - - return c; } @Override diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java index d97750c1f45..9c0aa2ca2d7 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/DigestAuthenticator.java @@ -46,6 +46,7 @@ import org.eclipse.jetty.util.QuotedStringTokenizer; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.security.Credential; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -282,6 +283,7 @@ public class DigestAuthenticator extends LoginAuthenticator private static class Nonce { + private final AutoLock _lock = new AutoLock(); final String _nonce; final long _ts; final BitSet _seen; @@ -295,7 +297,7 @@ public class DigestAuthenticator extends LoginAuthenticator public boolean seen(int count) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (count >= _seen.size()) return true; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java index 5aa7e3f1fa9..f45210e989a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractConnector.java @@ -33,7 +33,6 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; -import java.util.concurrent.locks.Condition; import java.util.stream.Collectors; import org.eclipse.jetty.io.ArrayByteBufferPool; @@ -143,8 +142,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co { protected static final Logger LOG = LoggerFactory.getLogger(AbstractConnector.class); - private final AutoLock _lock = new AutoLock(); - private final Condition _setAccepting = _lock.newCondition(); + private final AutoLock.WithCondition _lock = new AutoLock.WithCondition(); private final Map _factories = new LinkedHashMap<>(); // Order is important on server side, so we use a LinkedHashMap private final Server _server; private final Executor _executor; @@ -325,8 +323,10 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co return false; for (Thread a : _acceptors) + { if (a != null) return false; + } return true; } @@ -446,10 +446,10 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co public void setAccepting(boolean accepting) { - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { _accepting = accepting; - _setAccepting.signalAll(); + lock.signalAll(); } } @@ -702,17 +702,17 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co if (_acceptorPriorityDelta != 0) thread.setPriority(Math.max(Thread.MIN_PRIORITY, Math.min(Thread.MAX_PRIORITY, priority + _acceptorPriorityDelta))); - _acceptors[_id] = thread; + _lock.runLocked(() -> _acceptors[_id] = thread); try { while (isRunning() && !_shutdown.isShutdown()) { - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { if (!_accepting && isRunning()) { - _setAccepting.await(); + lock.await(); continue; } } @@ -738,10 +738,8 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co if (_acceptorPriorityDelta != 0) thread.setPriority(priority); - synchronized (AbstractConnector.this) - { - _acceptors[_id] = null; - } + _lock.runLocked(() -> _acceptors[_id] = null); + Shutdown shutdown = _shutdown; if (shutdown != null) shutdown.check(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AcceptRateLimit.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AcceptRateLimit.java index 28d9ca77db4..c8e69d34db0 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AcceptRateLimit.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AcceptRateLimit.java @@ -31,6 +31,7 @@ import org.eclipse.jetty.util.annotation.Name; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.component.Container; import org.eclipse.jetty.util.statistic.RateStatistic; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Scheduler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -64,6 +65,7 @@ public class AcceptRateLimit extends AbstractLifeCycle implements SelectorManage { private static final Logger LOG = LoggerFactory.getLogger(AcceptRateLimit.class); + private final AutoLock _lock = new AutoLock(); private final Server _server; private final List _connectors = new ArrayList<>(); private final Rate _rate; @@ -123,7 +125,7 @@ public class AcceptRateLimit extends AbstractLifeCycle implements SelectorManage @ManagedOperation(value = "Resets the accept rate", impact = "ACTION") public void reset() { - synchronized (_rate) + try (AutoLock ignored = _lock.lock()) { _rate.reset(); if (_limiting) @@ -142,7 +144,7 @@ public class AcceptRateLimit extends AbstractLifeCycle implements SelectorManage @Override protected void doStart() throws Exception { - synchronized (_rate) + try (AutoLock ignored = _lock.lock()) { if (_server != null) { @@ -156,7 +158,7 @@ public class AcceptRateLimit extends AbstractLifeCycle implements SelectorManage } if (LOG.isDebugEnabled()) - LOG.debug("AcceptLimit accept<{} rate<{} in {} for {}", _acceptRateLimit, _rate, _connectors); + LOG.debug("AcceptLimit accept<{} rate<{} in {}", _acceptRateLimit, _rate, _connectors); for (AbstractConnector c : _connectors) { @@ -168,7 +170,7 @@ public class AcceptRateLimit extends AbstractLifeCycle implements SelectorManage @Override protected void doStop() throws Exception { - synchronized (_rate) + try (AutoLock ignored = _lock.lock()) { if (_task != null) _task.cancel(); @@ -203,19 +205,16 @@ public class AcceptRateLimit extends AbstractLifeCycle implements SelectorManage @Override public void onAccepting(SelectableChannel channel) { - synchronized (_rate) + try (AutoLock ignored = _lock.lock()) { int rate = _rate.record(); if (LOG.isDebugEnabled()) - { LOG.debug("onAccepting rate {}/{} for {} {}", rate, _acceptRateLimit, _rate, channel); - } if (rate > _acceptRateLimit) { if (!_limiting) { _limiting = true; - LOG.warn("AcceptLimit rate exceeded {}>{} on {}", rate, _acceptRateLimit, _connectors); limit(); } @@ -238,7 +237,7 @@ public class AcceptRateLimit extends AbstractLifeCycle implements SelectorManage @Override public void run() { - synchronized (_rate) + try (AutoLock ignored = _lock.lock()) { _task = null; if (!isRunning()) @@ -258,7 +257,7 @@ public class AcceptRateLimit extends AbstractLifeCycle implements SelectorManage } } - private final class Rate extends RateStatistic + private static final class Rate extends RateStatistic { private Rate(long period, TimeUnit units) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncRequestLogWriter.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncRequestLogWriter.java index f1a9ba6cb88..a6f78640735 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncRequestLogWriter.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncRequestLogWriter.java @@ -92,7 +92,7 @@ public class AsyncRequestLogWriter extends RequestLogWriter } @Override - protected synchronized void doStart() throws Exception + protected void doStart() throws Exception { super.doStart(); _thread = new AsyncRequestLogWriter.WriterThread(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ConnectionLimit.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ConnectionLimit.java index 054923a2862..1136330806b 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ConnectionLimit.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ConnectionLimit.java @@ -33,6 +33,7 @@ import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.Name; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.component.Container; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -62,9 +63,10 @@ public class ConnectionLimit extends AbstractLifeCycle implements Listener, Sele { private static final Logger LOG = LoggerFactory.getLogger(ConnectionLimit.class); - private final Server _server; + private final AutoLock _lock = new AutoLock(); private final List _connectors = new ArrayList<>(); private final Set _accepting = new HashSet<>(); + private final Server _server; private int _connections; private int _maxConnections; private long _idleTimeout; @@ -108,33 +110,24 @@ public class ConnectionLimit extends AbstractLifeCycle implements Listener, Sele @ManagedAttribute("The maximum number of connections allowed") public int getMaxConnections() { - synchronized (this) - { - return _maxConnections; - } + return _lock.runLocked(() -> _maxConnections); } public void setMaxConnections(int max) { - synchronized (this) - { - _maxConnections = max; - } + _lock.runLocked(() -> _maxConnections = max); } @ManagedAttribute("The current number of connections ") public int getConnections() { - synchronized (this) - { - return _connections; - } + return _lock.runLocked(() -> _connections); } @Override protected void doStart() throws Exception { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_server != null) { @@ -160,7 +153,7 @@ public class ConnectionLimit extends AbstractLifeCycle implements Listener, Sele @Override protected void doStop() throws Exception { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { for (AbstractConnector c : _connectors) { @@ -229,7 +222,7 @@ public class ConnectionLimit extends AbstractLifeCycle implements Listener, Sele @Override public void onAccepting(SelectableChannel channel) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _accepting.add(channel); if (LOG.isDebugEnabled()) @@ -241,7 +234,7 @@ public class ConnectionLimit extends AbstractLifeCycle implements Listener, Sele @Override public void onAcceptFailed(SelectableChannel channel, Throwable cause) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _accepting.remove(channel); if (LOG.isDebugEnabled()) @@ -258,7 +251,7 @@ public class ConnectionLimit extends AbstractLifeCycle implements Listener, Sele @Override public void onOpened(Connection connection) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _accepting.remove(connection.getEndPoint().getTransport()); _connections++; @@ -271,7 +264,7 @@ public class ConnectionLimit extends AbstractLifeCycle implements Listener, Sele @Override public void onClosed(Connection connection) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _connections--; if (LOG.isDebugEnabled()) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java index e67ad99993d..3126306e863 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java @@ -408,7 +408,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog * @see org.eclipse.jetty.util.component.AbstractLifeCycle#doStart() */ @Override - protected synchronized void doStart() throws Exception + protected void doStart() throws Exception { if (_ignorePaths != null && _ignorePaths.length > 0) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java index 52bf1ab2904..7fd6830b42f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.server; import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; import javax.servlet.AsyncListener; import javax.servlet.ServletContext; import javax.servlet.ServletResponse; @@ -32,6 +33,7 @@ import org.eclipse.jetty.io.QuietException; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.ContextHandler.Context; import org.eclipse.jetty.server.handler.ErrorHandler; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Scheduler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -144,6 +146,7 @@ public class HttpChannelState WAIT, // Wait for further events } + private final AutoLock _lock = new AutoLock(); private final HttpChannel _channel; private List _asyncListeners; private State _state = State.IDLE; @@ -161,17 +164,24 @@ public class HttpChannelState _channel = channel; } + AutoLock lock() + { + return _lock.lock(); + } + + R runLocked(Supplier code) + { + return _lock.runLocked(code); + } + public State getState() { - synchronized (this) - { - return _state; - } + return runLocked(() -> _state); } public void addListener(AsyncListener listener) { - synchronized (this) + try (AutoLock ignored = lock()) { if (_asyncListeners == null) _asyncListeners = new ArrayList<>(); @@ -181,7 +191,7 @@ public class HttpChannelState public boolean hasListener(AsyncListener listener) { - synchronized (this) + try (AutoLock ignored = lock()) { if (_asyncListeners == null) return false; @@ -200,43 +210,28 @@ public class HttpChannelState public boolean isSendError() { - synchronized (this) - { - return _sendError; - } + return runLocked(() -> _sendError); } public void setTimeout(long ms) { - synchronized (this) - { - _timeoutMs = ms; - } + runLocked(() -> _timeoutMs = ms); } public long getTimeout() { - synchronized (this) - { - return _timeoutMs; - } + return runLocked(() -> _timeoutMs); } public AsyncContextEvent getAsyncContextEvent() { - synchronized (this) - { - return _event; - } + return runLocked(() -> _event); } @Override public String toString() { - synchronized (this) - { - return toStringLocked(); - } + return runLocked(this::toStringLocked); } private String toStringLocked() @@ -262,15 +257,12 @@ public class HttpChannelState public String getStatusString() { - synchronized (this) - { - return getStatusStringLocked(); - } + return runLocked(this::getStatusStringLocked); } public boolean commitResponse() { - synchronized (this) + try (AutoLock ignored = lock()) { switch (_outputState) { @@ -286,7 +278,7 @@ public class HttpChannelState public boolean partialResponse() { - synchronized (this) + try (AutoLock ignored = lock()) { switch (_outputState) { @@ -302,7 +294,7 @@ public class HttpChannelState public boolean completeResponse() { - synchronized (this) + try (AutoLock ignored = lock()) { switch (_outputState) { @@ -319,7 +311,7 @@ public class HttpChannelState public boolean isResponseCommitted() { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { switch (_outputState) { @@ -333,15 +325,12 @@ public class HttpChannelState public boolean isResponseCompleted() { - synchronized (this) - { - return _outputState == OutputState.COMPLETED; - } + return runLocked(() -> _outputState == OutputState.COMPLETED); } public boolean abortResponse() { - synchronized (this) + try (AutoLock ignored = lock()) { switch (_outputState) { @@ -365,7 +354,7 @@ public class HttpChannelState */ public Action handling() { - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("handling {}", toStringLocked()); @@ -407,7 +396,7 @@ public class HttpChannelState */ protected Action unhandle() { - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("unhandle {}", toStringLocked()); @@ -526,7 +515,7 @@ public class HttpChannelState { final List lastAsyncListeners; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("startAsync {}", toStringLocked()); @@ -575,7 +564,7 @@ public class HttpChannelState { boolean dispatch = false; AsyncContextEvent event; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("dispatch {} -> {}", toStringLocked(), path); @@ -611,7 +600,7 @@ public class HttpChannelState protected void timeout() { boolean dispatch = false; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("Timeout {}", toStringLocked()); @@ -639,7 +628,7 @@ public class HttpChannelState { final List listeners; AsyncContextEvent event; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("onTimeout {}", toStringLocked()); @@ -687,7 +676,7 @@ public class HttpChannelState { boolean handle = false; AsyncContextEvent event; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("complete {}", toStringLocked()); @@ -725,7 +714,7 @@ public class HttpChannelState // actually handled by #thrownException AsyncContextEvent event = null; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("asyncError " + toStringLocked(), failure); @@ -756,7 +745,7 @@ public class HttpChannelState { final AsyncContextEvent asyncEvent; final List asyncListeners; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("thrownException " + getStatusStringLocked(), th); @@ -820,7 +809,7 @@ public class HttpChannelState }); // check the actions of the listeners - synchronized (this) + try (AutoLock ignored = lock()) { if (_requestState == RequestState.ASYNC && !_sendError) { @@ -892,7 +881,7 @@ public class HttpChannelState if (message == null) message = HttpStatus.getMessage(code); - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("sendError {}", toStringLocked()); @@ -933,7 +922,7 @@ public class HttpChannelState protected void completing() { - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("completing {}", toStringLocked()); @@ -954,7 +943,7 @@ public class HttpChannelState final AsyncContextEvent event; boolean handle = false; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("completed {}", toStringLocked()); @@ -1008,7 +997,7 @@ public class HttpChannelState } event.completed(); - synchronized (this) + try (AutoLock ignored = lock()) { _requestState = RequestState.COMPLETED; if (_state == State.WAITING) @@ -1026,7 +1015,7 @@ public class HttpChannelState protected void recycle() { cancelTimeout(); - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("recycle {}", toStringLocked()); @@ -1055,7 +1044,7 @@ public class HttpChannelState public void upgrade() { cancelTimeout(); - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("upgrade {}", toStringLocked()); @@ -1085,11 +1074,7 @@ public class HttpChannelState protected void cancelTimeout() { - final AsyncContextEvent event; - synchronized (this) - { - event = _event; - } + AsyncContextEvent event = runLocked(() -> _event); cancelTimeout(event); } @@ -1101,48 +1086,33 @@ public class HttpChannelState public boolean isIdle() { - synchronized (this) - { - return _state == State.IDLE; - } + return runLocked(() -> _state == State.IDLE); } public boolean isExpired() { - synchronized (this) - { - // TODO review - return _requestState == RequestState.EXPIRE || _requestState == RequestState.EXPIRING; - } + // TODO review + return runLocked(() -> _requestState == RequestState.EXPIRE || _requestState == RequestState.EXPIRING); } public boolean isInitial() { - synchronized (this) - { - return _initial; - } + return runLocked(() -> _initial); } public boolean isSuspended() { - synchronized (this) - { - return _state == State.WAITING || _state == State.HANDLING && _requestState == RequestState.ASYNC; - } + return runLocked(() -> _state == State.WAITING || _state == State.HANDLING && _requestState == RequestState.ASYNC); } boolean isCompleted() { - synchronized (this) - { - return _requestState == RequestState.COMPLETED; - } + return runLocked(() -> _requestState == RequestState.COMPLETED); } public boolean isAsyncStarted() { - synchronized (this) + try (AutoLock ignored = lock()) { if (_state == State.HANDLING) return _requestState != RequestState.BLOCKING; @@ -1152,10 +1122,7 @@ public class HttpChannelState public boolean isAsync() { - synchronized (this) - { - return !_initial || _requestState != RequestState.BLOCKING; - } + return runLocked(() -> !_initial || _requestState != RequestState.BLOCKING); } public Request getBaseRequest() @@ -1170,12 +1137,7 @@ public class HttpChannelState public ContextHandler getContextHandler() { - final AsyncContextEvent event; - synchronized (this) - { - event = _event; - } - return getContextHandler(event); + return getContextHandler(getAsyncContextEvent()); } ContextHandler getContextHandler(AsyncContextEvent event) @@ -1191,12 +1153,7 @@ public class HttpChannelState public ServletResponse getServletResponse() { - final AsyncContextEvent event; - synchronized (this) - { - event = _event; - } - return getServletResponse(event); + return getServletResponse(getAsyncContextEvent()); } public ServletResponse getServletResponse(AsyncContextEvent event) @@ -1240,7 +1197,7 @@ public class HttpChannelState public void onReadUnready() { boolean interested = false; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("onReadUnready {}", toStringLocked()); @@ -1286,7 +1243,7 @@ public class HttpChannelState public boolean onContentAdded() { boolean woken = false; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("onContentAdded {}", toStringLocked()); @@ -1329,7 +1286,7 @@ public class HttpChannelState public boolean onReadReady() { boolean woken = false; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("onReadReady {}", toStringLocked()); @@ -1362,7 +1319,7 @@ public class HttpChannelState public boolean onReadPossible() { boolean woken = false; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("onReadPossible {}", toStringLocked()); @@ -1394,7 +1351,7 @@ public class HttpChannelState public boolean onReadEof() { boolean woken = false; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("onEof {}", toStringLocked()); @@ -1414,7 +1371,7 @@ public class HttpChannelState { boolean wake = false; - synchronized (this) + try (AutoLock ignored = lock()) { if (LOG.isDebugEnabled()) LOG.debug("onWritePossible {}", toStringLocked()); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java index ed4f6bd240b..34d7e630299 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java @@ -36,6 +36,7 @@ import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.component.Destroyable; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -125,6 +126,7 @@ public class HttpInput extends ServletInputStream implements Runnable static final Content EOF_CONTENT = new EofContent("EOF"); static final Content EARLY_EOF_CONTENT = new EofContent("EARLY_EOF"); + private final AutoLock.WithCondition _lock = new AutoLock.WithCondition(); private final byte[] _oneByteBuffer = new byte[1]; private Content _content; private Content _intercepted; @@ -151,7 +153,7 @@ public class HttpInput extends ServletInputStream implements Runnable public void recycle() { - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { if (_content != null) _content.failed(null); @@ -212,7 +214,7 @@ public class HttpInput extends ServletInputStream implements Runnable { int available = 0; boolean woken = false; - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { if (_content == null) _content = _inputQ.poll(); @@ -260,7 +262,7 @@ public class HttpInput extends ServletInputStream implements Runnable { boolean wake = false; int l; - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { // Calculate minimum request rate for DOS protection long minRequestDataRate = _channelState.getHttpChannel().getHttpConfiguration().getMinRequestDataRate(); @@ -333,7 +335,7 @@ public class HttpInput extends ServletInputStream implements Runnable */ public void asyncReadProduce() throws IOException { - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { produceContent(); } @@ -539,9 +541,9 @@ public class HttpInput extends ServletInputStream implements Runnable if (LOG.isDebugEnabled()) LOG.debug("{} blocking for content timeout={}", this, timeout); if (timeout > 0) - _inputQ.wait(timeout); + _lock.await(timeout, TimeUnit.MILLISECONDS); else - _inputQ.wait(); + _lock.await(); loop = true; } @@ -560,7 +562,7 @@ public class HttpInput extends ServletInputStream implements Runnable */ public boolean addContent(Content content) { - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { _waitingForContent = false; if (_firstByteTimeStamp == -1) @@ -594,26 +596,20 @@ public class HttpInput extends ServletInputStream implements Runnable public boolean hasContent() { - synchronized (_inputQ) - { - return _content != null || _inputQ.size() > 0; - } + return _lock.runLocked(() -> _content != null || _inputQ.size() > 0); } public void unblock() { - synchronized (_inputQ) + try (AutoLock.WithCondition lock = _lock.lock()) { - _inputQ.notify(); + lock.signal(); } } public long getContentConsumed() { - synchronized (_inputQ) - { - return _contentConsumed; - } + return _lock.runLocked(() -> _contentConsumed); } /** @@ -640,7 +636,7 @@ public class HttpInput extends ServletInputStream implements Runnable public boolean consumeAll() { - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { try { @@ -669,27 +665,18 @@ public class HttpInput extends ServletInputStream implements Runnable public boolean isError() { - synchronized (_inputQ) - { - return _state instanceof ErrorState; - } + return _lock.runLocked(() -> _state instanceof ErrorState); } public boolean isAsync() { - synchronized (_inputQ) - { - return _state == ASYNC; - } + return _lock.runLocked(() -> _state == ASYNC); } @Override public boolean isFinished() { - synchronized (_inputQ) - { - return _state instanceof EOFState; - } + return _lock.runLocked(() -> _state instanceof EOFState); } @Override @@ -697,7 +684,7 @@ public class HttpInput extends ServletInputStream implements Runnable { try { - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { if (_listener == null) return true; @@ -725,7 +712,7 @@ public class HttpInput extends ServletInputStream implements Runnable boolean woken = false; try { - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { if (_listener != null) throw new IllegalStateException("ReadListener already set"); @@ -773,7 +760,7 @@ public class HttpInput extends ServletInputStream implements Runnable public boolean onIdleTimeout(Throwable x) { - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { boolean neverDispatched = getHttpChannelState().isIdle(); if ((_waitingForContent || neverDispatched) && !isError()) @@ -788,7 +775,7 @@ public class HttpInput extends ServletInputStream implements Runnable public boolean failed(Throwable x) { - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { // Errors may be reported multiple times, for example // a local idle timeout and a remote I/O failure. @@ -818,7 +805,7 @@ public class HttpInput extends ServletInputStream implements Runnable { if (_listener != null) return _channelState.onContentAdded(); - _inputQ.notify(); + _lock.signal(); return false; } @@ -833,7 +820,7 @@ public class HttpInput extends ServletInputStream implements Runnable Throwable error; boolean aeof = false; - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { listener = _listener; @@ -922,7 +909,7 @@ public class HttpInput extends ServletInputStream implements Runnable long consumed; int q; Content content; - synchronized (_inputQ) + try (AutoLock ignored = _lock.lock()) { state = _state; consumed = _contentConsumed; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java index 3101ed88bbe..70c3c5f5a73 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java @@ -44,6 +44,7 @@ import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.IteratingCallback; import org.eclipse.jetty.util.SharedBlockingCallback; import org.eclipse.jetty.util.SharedBlockingCallback.Blocker; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -238,10 +239,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable public void reopen() { - synchronized (_channelState) - { - _softClose = false; - } + _channelState.runLocked(() -> _softClose = false); } protected Blocker acquireWriteBlockingCallback() throws IOException @@ -278,7 +276,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable boolean wake = false; Callback closedCallback = null; ByteBuffer closeContent = null; - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { if (LOG.isDebugEnabled()) state = stateString(); @@ -381,10 +379,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable public void softClose() { - synchronized (_channelState) - { - _softClose = true; - } + _channelState.runLocked(() -> _softClose = true); } public void complete(Callback callback) @@ -397,7 +392,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable boolean succeeded = false; Throwable error = null; ByteBuffer content = null; - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { switch (_state) { @@ -473,7 +468,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable */ public void completed(Throwable failure) { - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { _state = State.CLOSED; releaseBuffer(failure); @@ -485,7 +480,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable { ByteBuffer content = null; Blocker blocker = null; - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { if (_onError != null) { @@ -602,10 +597,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable public ByteBuffer getBuffer() { - synchronized (_channelState) - { - return acquireBuffer(); - } + return _channelState.runLocked(this::acquireBuffer); } private ByteBuffer acquireBuffer() @@ -630,15 +622,12 @@ public class HttpOutput extends ServletOutputStream implements Runnable public boolean isClosed() { - synchronized (_channelState) - { - return _softClose || (_state != State.OPEN); - } + return _channelState.runLocked(() -> _softClose || (_state != State.OPEN)); } public boolean isAsync() { - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { switch (_apiState) { @@ -657,7 +646,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable public void flush() throws IOException { ByteBuffer content = null; - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { switch (_state) { @@ -740,7 +729,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable // Async or Blocking ? boolean async; - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { checkWritable(); long written = _written + len; @@ -871,7 +860,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable // Async or Blocking ? boolean async; - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { checkWritable(); long written = _written + len; @@ -946,7 +935,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable // Async or Blocking ? boolean async = false; - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { checkWritable(); long written = _written + 1; @@ -1209,7 +1198,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable private boolean prepareSendContent(int len, Callback callback) { - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { if (BufferUtil.hasContent(_aggregate)) { @@ -1347,7 +1336,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable public void recycle() { - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { _state = State.OPEN; _apiState = ApiState.BLOCKING; @@ -1370,7 +1359,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable public void resetBuffer() { - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { _interceptor.resetBuffer(); if (BufferUtil.hasContent(_aggregate)) @@ -1385,7 +1374,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable if (!_channel.getState().isAsync()) throw new IllegalStateException("!ASYNC: " + stateString()); boolean wake; - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { if (_apiState != ApiState.BLOCKING) throw new IllegalStateException("!OPEN" + stateString()); @@ -1400,7 +1389,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable @Override public boolean isReady() { - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { switch (_apiState) { @@ -1431,7 +1420,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable { Throwable error = null; - synchronized (_channelState) + try (AutoLock ignored = _channelState.lock()) { if (_onError != null) { @@ -1482,10 +1471,7 @@ public class HttpOutput extends ServletOutputStream implements Runnable @Override public String toString() { - synchronized (_channelState) - { - return String.format("%s@%x{%s}", this.getClass().getSimpleName(), hashCode(), stateString()); - } + return _channelState.runLocked(() -> String.format("%s@%x{%s}", this.getClass().getSimpleName(), hashCode(), stateString())); } private abstract class ChannelWriteCB extends IteratingCallback diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java b/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java index 12b9555fbad..91101c544f6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartFormInputStream.java @@ -45,6 +45,7 @@ import org.eclipse.jetty.util.MultiException; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.QuotedStringTokenizer; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -90,6 +91,8 @@ public class MultiPartFormInputStream } private static final Logger LOG = LoggerFactory.getLogger(MultiPartFormInputStream.class); + + private final AutoLock _lock = new AutoLock(); private final MultiMap _parts = new MultiMap<>(); private final InputStream _in; private final MultipartConfigElement _config; @@ -100,7 +103,7 @@ public class MultiPartFormInputStream private volatile boolean _deleteOnExit; private volatile boolean _writeFilesWithFilenames; private volatile int _bufferSize = 16 * 1024; - private State state = State.UNPARSED; + private State _state = State.UNPARSED; public class MultiPart implements Part { @@ -375,7 +378,7 @@ public class MultiPartFormInputStream if (((ServletInputStream)in).isFinished()) { _in = null; - state = State.PARSED; + _state = State.PARSED; return; } } @@ -407,24 +410,24 @@ public class MultiPartFormInputStream */ public void deleteParts() { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { - switch (state) + switch (_state) { case DELETED: case DELETING: return; case PARSING: - state = State.DELETING; + _state = State.DELETING; return; case UNPARSED: - state = State.DELETED; + _state = State.DELETED; return; case PARSED: - state = State.DELETED; + _state = State.DELETED; break; } } @@ -510,19 +513,19 @@ public class MultiPartFormInputStream */ protected void parse() { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { - switch (state) + switch (_state) { case UNPARSED: - state = State.PARSING; + _state = State.PARSING; break; case PARSED: return; default: - _err = new IOException(state.name()); + _err = new IOException(_state.name()); return; } } @@ -563,11 +566,11 @@ public class MultiPartFormInputStream while (true) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { - if (state != State.PARSING) + if (_state != State.PARSING) { - _err = new IOException(state.name()); + _err = new IOException(_state.name()); return; } } @@ -629,21 +632,21 @@ public class MultiPartFormInputStream finally { boolean cleanup = false; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { - switch (state) + switch (_state) { case PARSING: - state = State.PARSED; + _state = State.PARSED; break; case DELETING: - state = State.DELETED; + _state = State.DELETED; cleanup = true; break; default: - _err = new IllegalStateException(state.name()); + _err = new IllegalStateException(_state.name()); } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/RequestLogWriter.java b/jetty-server/src/main/java/org/eclipse/jetty/server/RequestLogWriter.java index b9c3b9e666d..2773ffdbe67 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/RequestLogWriter.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/RequestLogWriter.java @@ -28,6 +28,7 @@ import org.eclipse.jetty.util.RolloverFileOutputStream; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -39,6 +40,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr { private static final Logger LOG = LoggerFactory.getLogger(RequestLogWriter.class); + private final AutoLock _lock = new AutoLock(); private String _filename; private boolean _append; private int _retainDays; @@ -175,7 +177,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr @Override public void write(String requestEntry) throws IOException { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_writer == null) return; @@ -186,21 +188,21 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr } @Override - protected synchronized void doStart() throws Exception + protected void doStart() throws Exception { - if (_filename != null) - { - _fileOut = new RolloverFileOutputStream(_filename, _append, _retainDays, TimeZone.getTimeZone(getTimeZone()), _filenameDateFormat, null); - _closeOut = true; - LOG.info("Opened " + getDatedFilename()); - } - else - _fileOut = System.err; - - _out = _fileOut; - - synchronized (this) + try (AutoLock ignored = _lock.lock()) { + if (_filename != null) + { + _fileOut = new RolloverFileOutputStream(_filename, _append, _retainDays, TimeZone.getTimeZone(getTimeZone()), _filenameDateFormat, null); + _closeOut = true; + LOG.info("Opened " + getDatedFilename()); + } + else + { + _fileOut = System.err; + } + _out = _fileOut; _writer = new OutputStreamWriter(_out); } super.doStart(); @@ -220,9 +222,9 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr @Override protected void doStop() throws Exception { - synchronized (this) + super.doStop(); + try (AutoLock ignored = _lock.lock()) { - super.doStop(); try { if (_writer != null) @@ -233,6 +235,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr LOG.trace("IGNORED", e); } if (_out != null && _closeOut) + { try { _out.close(); @@ -241,6 +244,7 @@ public class RequestLogWriter extends AbstractLifeCycle implements RequestLog.Wr { LOG.trace("IGNORED", e); } + } _out = null; _fileOut = null; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java index f5b9addebcc..3be87aa28cb 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ResourceService.java @@ -377,22 +377,19 @@ public class ResourceService if (!endsWithSlash) { StringBuffer buf = request.getRequestURL(); - synchronized (buf) + int param = buf.lastIndexOf(";"); + if (param < 0) + buf.append('/'); + else + buf.insert(param, '/'); + String q = request.getQueryString(); + if (q != null && q.length() != 0) { - int param = buf.lastIndexOf(";"); - if (param < 0) - buf.append('/'); - else - buf.insert(param, '/'); - String q = request.getQueryString(); - if (q != null && q.length() != 0) - { - buf.append('?'); - buf.append(q); - } - response.setContentLength(0); - response.sendRedirect(response.encodeRedirectURL(buf.toString())); + buf.append('?'); + buf.append(q); } + response.setContentLength(0); + response.sendRedirect(response.encodeRedirectURL(buf.toString())); return; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java index 42477dcf50f..8d32b47ee70 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java @@ -37,6 +37,7 @@ import java.util.function.Predicate; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.component.Destroyable; import org.eclipse.jetty.util.component.LifeCycle; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ShutdownThread; /** @@ -85,6 +86,7 @@ public class ShutdownMonitor return getInstance().containsLifeCycle(lifeCycle); } + private final AutoLock.WithCondition _lock = new AutoLock.WithCondition(); private final Set _lifeCycles = new LinkedHashSet<>(); private boolean debug; private final String host; @@ -96,8 +98,9 @@ public class ShutdownMonitor /** * Creates a ShutdownMonitor using configuration from the System properties. *

- * STOP.PORT = the port to listen on (empty, null, or values less than 0 disable the stop ability)
- * STOP.KEY = the magic key/passphrase to allow the stop
+ * {@code STOP.PORT} = the port to listen on (empty, null, or values less than 0 disable the stop ability) + *
+ * {@code STOP.KEY} = the magic key/passphrase to allow the stop
*

* Note: server socket will only listen on localhost, and a successful stop will issue a System.exit() call. */ @@ -112,26 +115,17 @@ public class ShutdownMonitor private void addLifeCycles(LifeCycle... lifeCycles) { - synchronized (this) - { - _lifeCycles.addAll(Arrays.asList(lifeCycles)); - } + _lock.runLocked(() -> _lifeCycles.addAll(Arrays.asList(lifeCycles))); } private void removeLifeCycle(LifeCycle lifeCycle) { - synchronized (this) - { - _lifeCycles.remove(lifeCycle); - } + _lock.runLocked(() -> _lifeCycles.remove(lifeCycle)); } private boolean containsLifeCycle(LifeCycle lifeCycle) { - synchronized (this) - { - return _lifeCycles.contains(lifeCycle); - } + return _lock.runLocked(() -> _lifeCycles.contains(lifeCycle)); } private void debug(String format, Object... args) @@ -148,26 +142,17 @@ public class ShutdownMonitor public String getKey() { - synchronized (this) - { - return key; - } + return _lock.runLocked(() -> key); } public int getPort() { - synchronized (this) - { - return port; - } + return _lock.runLocked(() -> port); } public boolean isExitVm() { - synchronized (this) - { - return exitVm; - } + return _lock.runLocked(() -> exitVm); } public void setDebug(boolean flag) @@ -180,7 +165,7 @@ public class ShutdownMonitor */ public void setExitVm(boolean exitVm) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (alive) throw new IllegalStateException("ShutdownMonitor already started"); @@ -190,7 +175,7 @@ public class ShutdownMonitor public void setKey(String key) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (alive) throw new IllegalStateException("ShutdownMonitor already started"); @@ -200,7 +185,7 @@ public class ShutdownMonitor public void setPort(int port) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (alive) throw new IllegalStateException("ShutdownMonitor already started"); @@ -210,7 +195,7 @@ public class ShutdownMonitor protected void start() throws Exception { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (alive) { @@ -231,31 +216,28 @@ public class ShutdownMonitor private void stop() { - synchronized (this) + try (AutoLock.WithCondition lock = _lock.lock()) { alive = false; - notifyAll(); + lock.signalAll(); } } // For test purposes only. void await() throws InterruptedException { - synchronized (this) + try (AutoLock.WithCondition lock = _lock.lock()) { while (alive) { - wait(); + lock.await(); } } } protected boolean isAlive() { - synchronized (this) - { - return alive; - } + return _lock.runLocked(() -> alive); } private ServerSocket listen() @@ -429,12 +411,7 @@ public class ShutdownMonitor private void stopLifeCycles(Predicate predicate, boolean destroy) { - List lifeCycles = new ArrayList<>(); - synchronized (this) - { - lifeCycles.addAll(_lifeCycles); - } - + List lifeCycles = _lock.runLocked(() -> new ArrayList<>(_lifeCycles)); for (LifeCycle l : lifeCycles) { try diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Slf4jRequestLogWriter.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Slf4jRequestLogWriter.java index e5faf948df7..3dad95333a0 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Slf4jRequestLogWriter.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Slf4jRequestLogWriter.java @@ -63,7 +63,7 @@ public class Slf4jRequestLogWriter extends AbstractLifeCycle implements RequestL } @Override - protected synchronized void doStart() throws Exception + protected void doStart() throws Exception { logger = LoggerFactory.getLogger(loggerName); super.doStart(); 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 edf9a20139a..941bcacbee0 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 @@ -86,6 +86,7 @@ import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.DumpableCollection; import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.util.resource.Resource; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -128,19 +129,12 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu }; public static final int DEFAULT_LISTENER_TYPE_INDEX = 1; - public static final int EXTENDED_LISTENER_TYPE_INDEX = 0; - private static final String UNIMPLEMENTED_USE_SERVLET_CONTEXT_HANDLER = "Unimplemented {} - use org.eclipse.jetty.servlet.ServletContextHandler"; - private static final Logger LOG = LoggerFactory.getLogger(ContextHandler.class); - private static final ThreadLocal __context = new ThreadLocal<>(); - private static String __serverInfo = "jetty/" + Server.getVersion(); - public static final String MANAGED_ATTRIBUTES = "org.eclipse.jetty.server.context.ManagedAttributes"; - public static final String MAX_FORM_KEYS_KEY = "org.eclipse.jetty.server.Request.maxFormKeys"; public static final String MAX_FORM_CONTENT_SIZE_KEY = "org.eclipse.jetty.server.Request.maxFormContentSize"; public static final int DEFAULT_MAX_FORM_KEYS = 1000; @@ -182,7 +176,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu INITIALIZED, DESTROYED } - + + private final AutoLock _lock = new AutoLock(); protected ContextStatus _contextStatus = ContextStatus.NOTSET; protected Context _scontext; private final AttributesMap _attributes; @@ -209,7 +204,6 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu private int _maxFormContentSize = Integer.getInteger(MAX_FORM_CONTENT_SIZE_KEY, DEFAULT_MAX_FORM_CONTENT_SIZE); private boolean _compactPath = false; private boolean _usingSecurityManager = System.getSecurityManager() != null; - private final List _programmaticListeners = new CopyOnWriteArrayList<>(); private final List _servletContextListeners = new CopyOnWriteArrayList<>(); private final List _destroyServletContextListeners = new ArrayList<>(); @@ -220,15 +214,13 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu private final Set _durableListeners = new HashSet<>(); private String[] _protectedTargets; private final CopyOnWriteArrayList _aliasChecks = new CopyOnWriteArrayList<>(); + private Availability _availability = Availability.UNAVAILABLE; public enum Availability { UNAVAILABLE, STARTING, AVAILABLE, SHUTDOWN, } - ; - private volatile Availability _availability = Availability.UNAVAILABLE; - public ContextHandler() { this(null, null, null); @@ -521,7 +513,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public ClassLoader getClassLoader() { - return _classLoader; + return _lock.runLocked(() -> _classLoader); } /** @@ -532,9 +524,10 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu @ManagedAttribute("The file classpath") public String getClassPath() { - if (_classLoader == null || !(_classLoader instanceof URLClassLoader)) + ClassLoader classLoader = getClassLoader(); + if (!(classLoader instanceof URLClassLoader)) return null; - URLClassLoader loader = (URLClassLoader)_classLoader; + URLClassLoader loader = (URLClassLoader)classLoader; URL[] urls = loader.getURLs(); StringBuilder classpath = new StringBuilder(); for (int i = 0; i < urls.length; i++) @@ -746,7 +739,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public boolean isAvailable() { - return _availability == Availability.AVAILABLE; + return _lock.runLocked(() -> _availability == Availability.AVAILABLE); } /** @@ -756,7 +749,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void setAvailable(boolean available) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (available && isRunning()) _availability = Availability.AVAILABLE; @@ -797,14 +790,15 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu _durableListeners.addAll(getEventListeners()); + ClassLoader classLoader = getClassLoader(); try { // Set the classloader, context and enter scope - if (_classLoader != null) + if (classLoader != null) { currentThread = Thread.currentThread(); oldClassloader = currentThread.getContextClassLoader(); - currentThread.setContextClassLoader(_classLoader); + currentThread.setContextClassLoader(classLoader); } oldContext = __context.get(); __context.set(_scontext); @@ -825,7 +819,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu exitScope(null); __context.set(oldContext); // reset the classloader - if (_classLoader != null && currentThread != null) + if (classLoader != null && currentThread != null) currentThread.setContextClassLoader(oldClassloader); } } @@ -1002,12 +996,13 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu try { // Set the classloader - if (_classLoader != null) + ClassLoader classLoader = getClassLoader(); + if (classLoader != null) { - oldWebapploader = _classLoader; + oldWebapploader = classLoader; currentThread = Thread.currentThread(); oldClassloader = currentThread.getContextClassLoader(); - currentThread.setContextClassLoader(_classLoader); + currentThread.setContextClassLoader(classLoader); } stopContext(); @@ -1207,8 +1202,9 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } } - if (_classLoader != null) - currentThread.setContextClassLoader(_classLoader); + ClassLoader classLoader = getClassLoader(); + if (classLoader != null) + currentThread.setContextClassLoader(classLoader); try { @@ -1233,7 +1229,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu exitScope(baseRequest); // reset the classloader - if (_classLoader != null) + if (classLoader != null) currentThread.setContextClassLoader(oldClassloader); // reset the context and servlet path. @@ -1360,8 +1356,6 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void handle(Request request, Runnable runnable) { - ClassLoader oldClassloader = null; - Thread currentThread = null; Context oldContext = __context.get(); // Are we already in the scope? @@ -1372,16 +1366,18 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } // Nope, so enter the scope and then exit + ClassLoader classLoader = getClassLoader(); + ClassLoader oldClassLoader = null; try { __context.set(_scontext); // Set the classloader - if (_classLoader != null) + if (classLoader != null) { - currentThread = Thread.currentThread(); - oldClassloader = currentThread.getContextClassLoader(); - currentThread.setContextClassLoader(_classLoader); + Thread currentThread = Thread.currentThread(); + oldClassLoader = currentThread.getContextClassLoader(); + currentThread.setContextClassLoader(classLoader); } enterScope(request, runnable); @@ -1392,10 +1388,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu exitScope(request); __context.set(oldContext); - if (oldClassloader != null) - { - currentThread.setContextClassLoader(oldClassloader); - } + if (classLoader != null) + Thread.currentThread().setContextClassLoader(oldClassLoader); } } @@ -1502,7 +1496,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public void setClassLoader(ClassLoader classLoader) { - _classLoader = classLoader; + _lock.runLocked(() -> _classLoader = classLoader); } /** @@ -1783,15 +1777,16 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu return b.toString(); } - public synchronized Class loadClass(String className) throws ClassNotFoundException + public Class loadClass(String className) throws ClassNotFoundException { if (className == null) return null; - if (_classLoader == null) + ClassLoader classLoader = getClassLoader(); + if (classLoader == null) return Loader.loadClass(className); - return _classLoader.loadClass(className); + return classLoader.loadClass(className); } public void addLocaleEncoding(String locale, String encoding) @@ -2023,6 +2018,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu */ public class Context extends StaticContext { + private final AutoLock _lock = new AutoLock(); protected boolean _enabled = true; // whether or not the dynamic API is enabled for callers protected boolean _extendedListenerTypes = false; @@ -2260,70 +2256,79 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } @Override - public synchronized Object getAttribute(String name) + public Object getAttribute(String name) { - Object o = ContextHandler.this.getAttribute(name); - if (o == null) - o = super.getAttribute(name); - return o; + try (AutoLock ignored = _lock.lock()) + { + Object o = ContextHandler.this.getAttribute(name); + if (o == null) + o = super.getAttribute(name); + return o; + } } @Override - public synchronized Enumeration getAttributeNames() + public Enumeration getAttributeNames() { - HashSet set = new HashSet<>(); - Enumeration e = super.getAttributeNames(); - while (e.hasMoreElements()) + try (AutoLock ignored = _lock.lock()) { - set.add(e.nextElement()); - } - e = _attributes.getAttributeNames(); - while (e.hasMoreElements()) - { - set.add(e.nextElement()); - } - - return Collections.enumeration(set); - } - - @Override - public synchronized void setAttribute(String name, Object value) - { - Object oldValue = super.getAttribute(name); - - if (value == null) - super.removeAttribute(name); - else - super.setAttribute(name, value); - - if (!_servletContextAttributeListeners.isEmpty()) - { - ServletContextAttributeEvent event = new ServletContextAttributeEvent(_scontext, name, oldValue == null ? value : oldValue); - - for (ServletContextAttributeListener l : _servletContextAttributeListeners) + HashSet set = new HashSet<>(); + Enumeration e = super.getAttributeNames(); + while (e.hasMoreElements()) { - if (oldValue == null) - l.attributeAdded(event); - else if (value == null) - l.attributeRemoved(event); - else - l.attributeReplaced(event); + set.add(e.nextElement()); + } + e = ContextHandler.this.getAttributeNames(); + while (e.hasMoreElements()) + { + set.add(e.nextElement()); + } + return Collections.enumeration(set); + } + } + + @Override + public void setAttribute(String name, Object value) + { + try (AutoLock ignored = _lock.lock()) + { + Object oldValue = super.getAttribute(name); + + if (value == null) + super.removeAttribute(name); + else + super.setAttribute(name, value); + + if (!_servletContextAttributeListeners.isEmpty()) + { + ServletContextAttributeEvent event = new ServletContextAttributeEvent(_scontext, name, oldValue == null ? value : oldValue); + for (ServletContextAttributeListener l : _servletContextAttributeListeners) + { + if (oldValue == null) + l.attributeAdded(event); + else if (value == null) + l.attributeRemoved(event); + else + l.attributeReplaced(event); + } } } } @Override - public synchronized void removeAttribute(String name) + public void removeAttribute(String name) { - Object oldValue = super.getAttribute(name); - super.removeAttribute(name); - if (oldValue != null && !_servletContextAttributeListeners.isEmpty()) + try (AutoLock ignored = _lock.lock()) { - ServletContextAttributeEvent event = new ServletContextAttributeEvent(_scontext, name, oldValue); - - for (ServletContextAttributeListener l : _servletContextAttributeListeners) + Object oldValue = super.getAttribute(name); + super.removeAttribute(name); + if (oldValue != null && !_servletContextAttributeListeners.isEmpty()) { - l.attributeRemoved(event); + ServletContextAttributeEvent event = new ServletContextAttributeEvent(_scontext, name, oldValue); + for (ServletContextAttributeListener l : _servletContextAttributeListeners) + { + l.attributeRemoved(event); + } } } } @@ -2366,9 +2371,11 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu try { - @SuppressWarnings( - {"unchecked", "rawtypes"}) - Class clazz = _classLoader == null ? Loader.loadClass(className) : (Class)_classLoader.loadClass(className); + ClassLoader classLoader = ContextHandler.this.getClassLoader(); + @SuppressWarnings("unchecked") + Class clazz = classLoader == null + ? Loader.loadClass(className) + : (Class)classLoader.loadClass(className); addListener(clazz); } catch (ClassNotFoundException e) @@ -2438,10 +2445,12 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu if (!_enabled) throw new UnsupportedOperationException(); + ClassLoader classLoader = ContextHandler.this.getClassLoader(); + // no security manager just return the classloader if (!isUsingSecurityManager()) { - return _classLoader; + return classLoader; } else { @@ -2454,13 +2463,13 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu ClassLoader callerLoader = caller.getCallerClassLoader(2); while (callerLoader != null) { - if (callerLoader == _classLoader) - return _classLoader; + if (callerLoader == classLoader) + return classLoader; else callerLoader = callerLoader.getParent(); } System.getSecurityManager().checkPermission(new RuntimePermission("getClassLoader")); - return _classLoader; + return classLoader; } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java index 1d35b1ea63b..1f76ee38da0 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java @@ -32,6 +32,7 @@ import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,11 +52,10 @@ import org.slf4j.LoggerFactory; public class DefaultSessionIdManager extends ContainerLifeCycle implements SessionIdManager { private static final Logger LOG = LoggerFactory.getLogger(DefaultSessionIdManager.class); - public static final String __NEW_SESSION_ID = "org.eclipse.jetty.server.newSessionId"; - protected static final AtomicLong COUNTER = new AtomicLong(); + private final AutoLock _lock = new AutoLock(); protected Random _random; protected boolean _weakRandom; protected String _workerName; @@ -166,7 +166,7 @@ public class DefaultSessionIdManager extends ContainerLifeCycle implements Sessi /** * @param random a random number generator for generating ids */ - public synchronized void setRandom(Random random) + public void setRandom(Random random) { _random = random; _weakRandom = false; @@ -229,7 +229,7 @@ public class DefaultSessionIdManager extends ContainerLifeCycle implements Sessi // pick a new unique ID! String id = null; - synchronized (_random) + try (AutoLock ignored = _lock.lock()) { while (id == null || id.length() == 0) { @@ -268,7 +268,7 @@ public class DefaultSessionIdManager extends ContainerLifeCycle implements Sessi if (!StringUtil.isBlank(_workerName)) id = _workerName + id; - id = id + Long.toString(COUNTER.getAndIncrement()); + id = id + COUNTER.getAndIncrement(); } } return id; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HouseKeeper.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HouseKeeper.java index 3179e7771f5..d8626ed7eda 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HouseKeeper.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HouseKeeper.java @@ -24,6 +24,7 @@ import org.eclipse.jetty.server.SessionIdManager; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; import org.eclipse.jetty.util.thread.Scheduler; import org.slf4j.Logger; @@ -38,8 +39,9 @@ import org.slf4j.LoggerFactory; public class HouseKeeper extends AbstractLifeCycle { private static final Logger LOG = LoggerFactory.getLogger(HouseKeeper.class); - public static final long DEFAULT_PERIOD_MS = 1000L * 60 * 10; + + private final AutoLock _lock = new AutoLock(); protected SessionIdManager _sessionIdManager; protected Scheduler _scheduler; protected Scheduler.Task _task; //scavenge task @@ -125,7 +127,7 @@ public class HouseKeeper extends AbstractLifeCycle */ protected void startScavenging() throws Exception { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_scheduler != null) { @@ -147,7 +149,7 @@ public class HouseKeeper extends AbstractLifeCycle */ protected void stopScavenging() throws Exception { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_task != null) { @@ -168,7 +170,7 @@ public class HouseKeeper extends AbstractLifeCycle @Override protected void doStop() throws Exception { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { stopScavenging(); _scheduler = null; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java index 2ab519ca2b7..89f953108bf 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java @@ -24,7 +24,6 @@ import java.util.Enumeration; import java.util.Iterator; import java.util.Set; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Condition; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSessionActivationListener; @@ -93,8 +92,7 @@ public class Session implements SessionHandler.SessionIf protected State _state = State.VALID; // state of the session:valid,invalid // or being invalidated - protected AutoLock _lock = new AutoLock(); - protected Condition _stateChangeCompleted = _lock.newCondition(); + protected final AutoLock.WithCondition _lock = new AutoLock.WithCondition(); protected boolean _resident = false; protected final SessionInactivityTimer _sessionInactivityTimer; @@ -123,7 +121,7 @@ public class Session implements SessionHandler.SessionIf long now = System.currentTimeMillis(); //handle what to do with the session after the timer expired getSessionHandler().sessionInactivityTimerExpired(Session.this, now); - try (AutoLock lock = Session.this.lock()) + try (AutoLock ignored = Session.this.lock()) { //grab the lock and check what happened to the session: if it didn't get evicted and //it hasn't expired, we need to reset the timer @@ -208,10 +206,7 @@ public class Session implements SessionHandler.SessionIf */ public long getRequests() { - try (AutoLock lock = _lock.lock()) - { - return _requests; - } + return _lock.runLocked(() -> _requests); } public void setExtendedId(String extendedId) @@ -221,15 +216,12 @@ public class Session implements SessionHandler.SessionIf protected void cookieSet() { - try (AutoLock lock = _lock.lock()) - { - _sessionData.setCookieSet(_sessionData.getAccessed()); - } + _lock.runLocked(() -> _sessionData.setCookieSet(_sessionData.getAccessed())); } protected void use() { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { _requests++; @@ -242,7 +234,7 @@ public class Session implements SessionHandler.SessionIf protected boolean access(long time) { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { if (!isValid() || !isResident()) return false; @@ -262,7 +254,7 @@ public class Session implements SessionHandler.SessionIf protected void complete() { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { _requests--; @@ -289,10 +281,7 @@ public class Session implements SessionHandler.SessionIf */ protected boolean isExpiredAt(long time) { - try (AutoLock lock = _lock.lock()) - { - return _sessionData.isExpiredAt(time); - } + return _lock.runLocked(() -> _sessionData.isExpiredAt(time)); } /** @@ -304,10 +293,7 @@ public class Session implements SessionHandler.SessionIf protected boolean isIdleLongerThan(int sec) { long now = System.currentTimeMillis(); - try (AutoLock lock = _lock.lock()) - { - return ((_sessionData.getAccessed() + (sec * 1000)) <= now); - } + return _lock.runLocked(() -> (_sessionData.getAccessed() + (sec * 1000)) <= now); } /** @@ -413,32 +399,23 @@ public class Session implements SessionHandler.SessionIf public boolean isValid() { - try (AutoLock lock = _lock.lock()) - { - return _state == State.VALID; - } + return _lock.runLocked(() -> _state == State.VALID); } public boolean isInvalid() { - try (AutoLock lock = _lock.lock()) - { - return _state == State.INVALID || _state == State.INVALIDATING; - } + return _lock.runLocked(() -> _state == State.INVALID || _state == State.INVALIDATING); } public long getCookieSetTime() { - try (AutoLock lock = _lock.lock()) - { - return _sessionData.getCookieSet(); - } + return _lock.runLocked(_sessionData::getCookieSet); } @Override public long getCreationTime() throws IllegalStateException { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { checkValidForRead(); return _sessionData.getCreated(); @@ -448,10 +425,7 @@ public class Session implements SessionHandler.SessionIf @Override public String getId() { - try (AutoLock lock = _lock.lock()) - { - return _sessionData.getId(); - } + return _lock.runLocked(_sessionData::getId); } public String getExtendedId() @@ -472,12 +446,10 @@ public class Session implements SessionHandler.SessionIf @Override public long getLastAccessedTime() { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { if (isInvalid()) - { throw new IllegalStateException("Session not valid"); - } return _sessionData.getLastAccessed(); } } @@ -493,7 +465,7 @@ public class Session implements SessionHandler.SessionIf @Override public void setMaxInactiveInterval(int secs) { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { _sessionData.setMaxInactiveMs((long)secs * 1000L); _sessionData.calcAndSetExpiry(); @@ -526,7 +498,7 @@ public class Session implements SessionHandler.SessionIf { long time = 0; - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { long remaining = _sessionData.getExpiry() - now; long maxInactive = _sessionData.getMaxInactiveMs(); @@ -587,7 +559,7 @@ public class Session implements SessionHandler.SessionIf @Override public int getMaxInactiveInterval() { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { long maxInactiveMs = _sessionData.getMaxInactiveMs(); return (int)(maxInactiveMs < 0 ? -1 : maxInactiveMs / 1000); @@ -655,7 +627,7 @@ public class Session implements SessionHandler.SessionIf @Override public Object getAttribute(String name) { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { checkValidForRead(); return _sessionData.getAttribute(name); @@ -666,7 +638,7 @@ public class Session implements SessionHandler.SessionIf @Deprecated(since = "Servlet API 2.2") public Object getValue(String name) { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { checkValidForRead(); return _sessionData.getAttribute(name); @@ -676,7 +648,7 @@ public class Session implements SessionHandler.SessionIf @Override public Enumeration getAttributeNames() { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { checkValidForRead(); final Iterator itor = _sessionData.getKeys().iterator(); @@ -716,7 +688,7 @@ public class Session implements SessionHandler.SessionIf @Deprecated(since = "Servlet API 2.2") public String[] getValueNames() throws IllegalStateException { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { checkValidForRead(); Iterator itor = _sessionData.getKeys().iterator(); @@ -735,7 +707,7 @@ public class Session implements SessionHandler.SessionIf public void setAttribute(String name, Object value) { Object old = null; - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { // if session is not valid, don't accept the set checkValidForWrite(); @@ -779,7 +751,7 @@ public class Session implements SessionHandler.SessionIf String id = null; String extendedId = null; - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { while (true) { @@ -792,7 +764,7 @@ public class Session implements SessionHandler.SessionIf case CHANGING: try { - _stateChangeCompleted.await(); + lock.await(); } catch (InterruptedException e) { @@ -815,7 +787,7 @@ public class Session implements SessionHandler.SessionIf String newId = _handler._sessionIdManager.renewSessionId(id, extendedId, request); - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { switch (_state) { @@ -833,7 +805,7 @@ public class Session implements SessionHandler.SessionIf setIdChanged(true); _state = State.VALID; - _stateChangeCompleted.signalAll(); + lock.signalAll(); break; case INVALID: @@ -905,9 +877,8 @@ public class Session implements SessionHandler.SessionIf { boolean result = false; - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { - while (true) { switch (_state) @@ -931,7 +902,7 @@ public class Session implements SessionHandler.SessionIf { if (LOG.isDebugEnabled()) LOG.debug("Session {} waiting for id change to complete", _sessionData.getId()); - _stateChangeCompleted.await(); + lock.await(); } catch (InterruptedException e) { @@ -964,7 +935,7 @@ public class Session implements SessionHandler.SessionIf */ protected void finishInvalidate() throws IllegalStateException { - try (AutoLock lock = _lock.lock()) + try (AutoLock.WithCondition lock = _lock.lock()) { try { @@ -994,7 +965,7 @@ public class Session implements SessionHandler.SessionIf // mark as invalid _state = State.INVALID; _handler.recordSessionTime(this); - _stateChangeCompleted.signalAll(); + lock.signalAll(); } } } @@ -1002,7 +973,7 @@ public class Session implements SessionHandler.SessionIf @Override public boolean isNew() throws IllegalStateException { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { checkValidForRead(); return _newSession; @@ -1011,18 +982,12 @@ public class Session implements SessionHandler.SessionIf public void setIdChanged(boolean changed) { - try (AutoLock lock = _lock.lock()) - { - _idChanged = changed; - } + _lock.runLocked(() -> _idChanged = changed); } public boolean isIdChanged() { - try (AutoLock lock = _lock.lock()) - { - return _idChanged; - } + return _lock.runLocked(() -> _idChanged); } @Override @@ -1053,7 +1018,7 @@ public class Session implements SessionHandler.SessionIf @Override public String toString() { - try (AutoLock lock = _lock.lock()) + try (AutoLock ignored = _lock.lock()) { return String.format("%s@%x{id=%s,x=%s,req=%d,res=%b}", getClass().getSimpleName(), diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java index 98c91bd0694..9e687844d6b 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java @@ -399,6 +399,7 @@ public class SessionHandler extends ScopedHandler _context = ContextHandler.getCurrentContext(); _loader = Thread.currentThread().getContextClassLoader(); + // Use a coarser lock to serialize concurrent start of many contexts. synchronized (server) { //Get a SessionDataStore and a SessionDataStore, falling back to in-memory sessions only diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ServerConnectorTimeoutTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ServerConnectorTimeoutTest.java index 0ec2c4ae857..0593da48421 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ServerConnectorTimeoutTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ServerConnectorTimeoutTest.java @@ -122,7 +122,7 @@ public class ServerConnectorTimeoutTest extends ConnectorTimeoutTest }); } - private synchronized String process(String content) throws IOException, InterruptedException + private String process(String content) throws IOException, InterruptedException { String request = "GET / HTTP/1.1\r\n" + "Host: localhost\r\n"; diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/BaseHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/BaseHolder.java index a365c7b93aa..084c37aefa4 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/BaseHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/BaseHolder.java @@ -28,6 +28,7 @@ import org.eclipse.jetty.util.Loader; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.component.Dumpable; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,6 +45,7 @@ public abstract class BaseHolder extends AbstractLifeCycle implements Dumpabl { private static final Logger LOG = LoggerFactory.getLogger(BaseHolder.class); + private final AutoLock _lock = new AutoLock(); private final Source _source; private Class _class; private String _className; @@ -60,6 +62,11 @@ public abstract class BaseHolder extends AbstractLifeCycle implements Dumpabl return _source; } + AutoLock lock() + { + return _lock.lock(); + } + /** * Do any setup necessary after starting * @@ -164,30 +171,36 @@ public abstract class BaseHolder extends AbstractLifeCycle implements Dumpabl } } - protected synchronized void setInstance(T instance) + protected void setInstance(T instance) { - _instance = instance; - if (instance == null) - setHeldClass(null); - else - setHeldClass((Class)instance.getClass()); + try (AutoLock ignored = lock()) + { + _instance = instance; + if (instance == null) + setHeldClass(null); + else + setHeldClass((Class)instance.getClass()); + } } - protected synchronized T getInstance() + protected T getInstance() { - return _instance; + return _lock.runLocked(() -> _instance); } - protected synchronized T createInstance() throws Exception + protected T createInstance() throws Exception { - ServletContext ctx = getServletContext(); - if (ctx == null) - return getHeldClass().getDeclaredConstructor().newInstance(); + try (AutoLock ignored = lock()) + { + ServletContext ctx = getServletContext(); + if (ctx == null) + return getHeldClass().getDeclaredConstructor().newInstance(); - if (ServletContextHandler.Context.class.isAssignableFrom(ctx.getClass())) - return ((ServletContextHandler.Context)ctx).createInstance(this); + if (ServletContextHandler.Context.class.isAssignableFrom(ctx.getClass())) + return ((ServletContextHandler.Context)ctx).createInstance(this); - return null; + return null; + } } public ServletContext getServletContext() @@ -215,9 +228,9 @@ public abstract class BaseHolder extends AbstractLifeCycle implements Dumpabl /** * @return True if this holder was created for a specific instance. */ - public synchronized boolean isInstance() + public boolean isInstance() { - return _instance != null; + return _lock.runLocked(() -> _instance != null); } @Override diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java index 515ef89f9bf..044c3a2a6b6 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/FilterHolder.java @@ -34,6 +34,7 @@ import javax.servlet.ServletException; import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.DumpableCollection; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -102,7 +103,7 @@ public class FilterHolder extends Holder @Override public void initialize() throws Exception { - synchronized (this) + try (AutoLock ignored = lock()) { if (_filter != null) return; @@ -133,16 +134,19 @@ public class FilterHolder extends Holder } @Override - protected synchronized Filter createInstance() throws Exception + protected Filter createInstance() throws Exception { - Filter filter = super.createInstance(); - if (filter == null) + try (AutoLock ignored = lock()) { - ServletContext context = getServletContext(); - if (context != null) - filter = context.createFilter(getHeldClass()); + Filter filter = super.createInstance(); + if (filter == null) + { + ServletContext context = getServletContext(); + if (context != null) + filter = context.createFilter(getHeldClass()); + } + return filter; } - return filter; } @Override @@ -175,7 +179,7 @@ public class FilterHolder extends Holder getServletHandler().destroyFilter(f); } - public synchronized void setFilter(Filter filter) + public void setFilter(Filter filter) { setInstance(filter); } diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Holder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Holder.java index dfd1a9b4420..b2afb37f8e0 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Holder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Holder.java @@ -29,6 +29,7 @@ import javax.servlet.ServletContext; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -98,11 +99,14 @@ public abstract class Holder extends BaseHolder } @Override - protected synchronized void setInstance(T instance) + protected void setInstance(T instance) { - super.setInstance(instance); - if (getName() == null) - setName(String.format("%s@%x", instance.getClass().getName(), instance.hashCode())); + try (AutoLock ignored = lock()) + { + super.setInstance(instance); + if (getName() == null) + setName(String.format("%s@%x", instance.getClass().getName(), instance.hashCode())); + } } public void destroyInstance(Object instance) diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java index 17ae35dea14..47bb32fc528 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/Invoker.java @@ -38,6 +38,7 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.HandlerWrapper; import org.eclipse.jetty.util.ArrayUtil; import org.eclipse.jetty.util.URIUtil; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -163,7 +164,7 @@ public class Invoker extends HttpServlet return; } - synchronized (_servletHandler) + try (AutoLock ignored = _servletHandler.lock()) { // find the entry for the invoker (me) _invokerEntry = _servletHandler.getMappedServlet(servletPath); @@ -175,7 +176,7 @@ public class Invoker extends HttpServlet if (entry != null && !entry.equals(_invokerEntry)) { // Use the holder - holder = (ServletHolder)entry.getServletHolder(); + holder = entry.getServletHolder(); } else { diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java index 3259392743a..754f9dba6a3 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ListenerHolder.java @@ -23,6 +23,7 @@ import javax.servlet.ServletContext; import javax.servlet.ServletException; import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.thread.AutoLock; /** * ListenerHolder @@ -105,17 +106,19 @@ public class ListenerHolder extends BaseHolder } @Override - protected synchronized EventListener createInstance() throws Exception + protected EventListener createInstance() throws Exception { - - EventListener listener = super.createInstance(); - if (listener == null) + try (AutoLock ignored = lock()) { - ServletContext ctx = getServletContext(); - if (ctx != null) - listener = ctx.createListener(getHeldClass()); + EventListener listener = super.createInstance(); + if (listener == null) + { + ServletContext ctx = getServletContext(); + if (ctx != null) + listener = ctx.createListener(getHeldClass()); + } + return listener; } - return listener; } @Override 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 3206f4a0d1a..5a6e63ed869 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 @@ -69,6 +69,7 @@ import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.DumpableCollection; import org.eclipse.jetty.util.component.LifeCycle; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -80,7 +81,7 @@ import org.slf4j.LoggerFactory; *

* This handler does not implement the full J2EE features and is intended to * be used directly when a full web application is not required. If a Web application is required, - * then this handler should be used as part of a org.eclipse.jetty.webapp.WebAppContext. + * then this handler should be used as part of a {@code org.eclipse.jetty.webapp.WebAppContext}. *

* Unless run as part of a {@link ServletContextHandler} or derivative, the {@link #initialize()} * method must be called manually after start(). @@ -90,6 +91,7 @@ public class ServletHandler extends ScopedHandler { private static final Logger LOG = LoggerFactory.getLogger(ServletHandler.class); + private final AutoLock _lock = new AutoLock(); private ServletContextHandler _contextHandler; private ServletContext _servletContext; private FilterHolder[] _filters = new FilterHolder[0]; @@ -102,36 +104,29 @@ public class ServletHandler extends ScopedHandler private boolean _ensureDefaultServlet = true; private IdentityService _identityService; private boolean _allowDuplicateMappings = false; - private ServletHolder[] _servlets = new ServletHolder[0]; private ServletMapping[] _servletMappings; private final Map _filterNameMap = new HashMap<>(); private List _filterPathMappings; private MultiMap _filterNameMappings; - private final Map _servletNameMap = new HashMap<>(); private PathMappings _servletPathMap; - private ListenerHolder[] _listeners = new ListenerHolder[0]; private boolean _initialized = false; - @SuppressWarnings("unchecked") protected final ConcurrentMap[] _chainCache = new ConcurrentMap[FilterMapping.ALL]; - @SuppressWarnings("unchecked") protected final Queue[] _chainLRU = new Queue[FilterMapping.ALL]; - /** - * Constructor. - */ - public ServletHandler() + AutoLock lock() { + return _lock.lock(); } @Override public boolean isDumpable(Object o) { - return !(o instanceof Holder || o instanceof BaseHolder || o instanceof FilterMapping || o instanceof ServletMapping); + return !(o instanceof BaseHolder || o instanceof FilterMapping || o instanceof ServletMapping); } @Override @@ -146,8 +141,7 @@ public class ServletHandler extends ScopedHandler } @Override - protected synchronized void doStart() - throws Exception + protected void doStart() throws Exception { ContextHandler.Context context = ContextHandler.getCurrentContext(); _servletContext = context == null ? new ContextHandler.StaticContext() : context; @@ -230,8 +224,7 @@ public class ServletHandler extends ScopedHandler } @Override - protected synchronized void doStop() - throws Exception + protected void doStop() throws Exception { super.doStop(); @@ -865,7 +858,7 @@ public class ServletHandler extends ScopedHandler try { - synchronized (this) + try (AutoLock ignored = lock()) { if (servlet != null && !containsServletHolder(servlet)) setServlets(ArrayUtil.addToArray(holders, servlet, ServletHolder.class)); @@ -893,7 +886,7 @@ public class ServletHandler extends ScopedHandler if (holder == null) return; - synchronized (this) + try (AutoLock ignored = lock()) { if (!containsServletHolder(holder)) setServlets(ArrayUtil.addToArray(getServlets(), holder, ServletHolder.class)); @@ -978,7 +971,7 @@ public class ServletHandler extends ScopedHandler try { - synchronized (this) + try (AutoLock ignored = lock()) { if (holder != null && !containsFilterHolder(holder)) setFilters(ArrayUtil.addToArray(holders, holder, FilterHolder.class)); @@ -1046,7 +1039,7 @@ public class ServletHandler extends ScopedHandler try { - synchronized (this) + try (AutoLock ignored = lock()) { if (holder != null && !containsFilterHolder(holder)) setFilters(ArrayUtil.addToArray(holders, holder, FilterHolder.class)); @@ -1075,7 +1068,7 @@ public class ServletHandler extends ScopedHandler { if (filter != null) { - synchronized (this) + try (AutoLock ignored = lock()) { if (!containsFilterHolder(filter)) setFilters(ArrayUtil.addToArray(getFilters(), filter, FilterHolder.class)); @@ -1095,7 +1088,7 @@ public class ServletHandler extends ScopedHandler if (filter == null) return; - synchronized (this) + try (AutoLock ignored = lock()) { if (!containsFilterHolder(filter)) setFilters(ArrayUtil.addToArray(getFilters(), filter, FilterHolder.class)); @@ -1244,177 +1237,183 @@ public class ServletHandler extends ScopedHandler return newMappings; } - protected synchronized void updateNameMappings() + protected void updateNameMappings() { - // update filter name map - _filterNameMap.clear(); - if (_filters != null) + try (AutoLock ignored = lock()) { - for (FilterHolder filter : _filters) + // update filter name map + _filterNameMap.clear(); + if (_filters != null) { - _filterNameMap.put(filter.getName(), filter); - filter.setServletHandler(this); + for (FilterHolder filter : _filters) + { + _filterNameMap.put(filter.getName(), filter); + filter.setServletHandler(this); + } } - } - // Map servlet names to holders - _servletNameMap.clear(); - if (_servlets != null) - { - // update the maps - for (ServletHolder servlet : _servlets) + // Map servlet names to holders + _servletNameMap.clear(); + if (_servlets != null) { - _servletNameMap.put(servlet.getName(), new MappedServlet(null, servlet)); - servlet.setServletHandler(this); + // update the maps + for (ServletHolder servlet : _servlets) + { + _servletNameMap.put(servlet.getName(), new MappedServlet(null, servlet)); + servlet.setServletHandler(this); + } } } } - protected synchronized void updateMappings() + protected void updateMappings() { - // update filter mappings - if (_filterMappings == null) + try (AutoLock ignored = lock()) { - _filterPathMappings = null; - _filterNameMappings = null; - } - else - { - _filterPathMappings = new ArrayList<>(); - _filterNameMappings = new MultiMap<>(); - for (FilterMapping filtermapping : _filterMappings) + // update filter mappings + if (_filterMappings == null) { - FilterHolder filterHolder = _filterNameMap.get(filtermapping.getFilterName()); - if (filterHolder == null) - throw new IllegalStateException("No filter named " + filtermapping.getFilterName()); - filtermapping.setFilterHolder(filterHolder); - if (filtermapping.getPathSpecs() != null) - _filterPathMappings.add(filtermapping); - - if (filtermapping.getServletNames() != null) - { - String[] names = filtermapping.getServletNames(); - for (String name : names) - { - if (name != null) - _filterNameMappings.add(name, filtermapping); - } - } + _filterPathMappings = null; + _filterNameMappings = null; } - } - - // Map servlet paths to holders - if (_servletMappings == null) - { - _servletPathMap = null; - } - else - { - PathMappings pm = new PathMappings<>(); - - //create a map of paths to set of ServletMappings that define that mapping - HashMap> sms = new HashMap<>(); - for (ServletMapping servletMapping : _servletMappings) + else { - String[] pathSpecs = servletMapping.getPathSpecs(); - if (pathSpecs != null) + _filterPathMappings = new ArrayList<>(); + _filterNameMappings = new MultiMap<>(); + for (FilterMapping filtermapping : _filterMappings) { - for (String pathSpec : pathSpecs) + FilterHolder filterHolder = _filterNameMap.get(filtermapping.getFilterName()); + if (filterHolder == null) + throw new IllegalStateException("No filter named " + filtermapping.getFilterName()); + filtermapping.setFilterHolder(filterHolder); + if (filtermapping.getPathSpecs() != null) + _filterPathMappings.add(filtermapping); + + if (filtermapping.getServletNames() != null) { - List mappings = sms.computeIfAbsent(pathSpec, k -> new ArrayList<>()); - mappings.add(servletMapping); - } - } - } - - //evaluate path to servlet map based on servlet mappings - for (String pathSpec : sms.keySet()) - { - //for each path, look at the mappings where it is referenced - //if a mapping is for a servlet that is not enabled, skip it - List mappings = sms.get(pathSpec); - - ServletMapping finalMapping = null; - for (ServletMapping mapping : mappings) - { - //Get servlet associated with the mapping and check it is enabled - ServletHolder servletHolder = getServlet(mapping.getServletName()); - if (servletHolder == null) - throw new IllegalStateException("No such servlet: " + mapping.getServletName()); - //if the servlet related to the mapping is not enabled, skip it from consideration - if (!servletHolder.isEnabled()) - continue; - - //only accept a default mapping if we don't have any other - if (finalMapping == null) - finalMapping = mapping; - else - { - //already have a candidate - only accept another one - //if the candidate is a default, or we're allowing duplicate mappings - if (finalMapping.isFromDefaultDescriptor()) - finalMapping = mapping; - else if (isAllowDuplicateMappings()) + String[] names = filtermapping.getServletNames(); + for (String name : names) { - LOG.warn("Multiple servlets map to path {}: {} and {}, choosing {}", pathSpec, finalMapping.getServletName(), mapping.getServletName(), mapping); - finalMapping = mapping; + if (name != null) + _filterNameMappings.add(name, filtermapping); } + } + } + } + + // Map servlet paths to holders + if (_servletMappings == null) + { + _servletPathMap = null; + } + else + { + PathMappings pm = new PathMappings<>(); + + //create a map of paths to set of ServletMappings that define that mapping + HashMap> sms = new HashMap<>(); + for (ServletMapping servletMapping : _servletMappings) + { + String[] pathSpecs = servletMapping.getPathSpecs(); + if (pathSpecs != null) + { + for (String pathSpec : pathSpecs) + { + List mappings = sms.computeIfAbsent(pathSpec, k -> new ArrayList<>()); + mappings.add(servletMapping); + } + } + } + + //evaluate path to servlet map based on servlet mappings + for (String pathSpec : sms.keySet()) + { + //for each path, look at the mappings where it is referenced + //if a mapping is for a servlet that is not enabled, skip it + List mappings = sms.get(pathSpec); + + ServletMapping finalMapping = null; + for (ServletMapping mapping : mappings) + { + //Get servlet associated with the mapping and check it is enabled + ServletHolder servletHolder = getServlet(mapping.getServletName()); + if (servletHolder == null) + throw new IllegalStateException("No such servlet: " + mapping.getServletName()); + //if the servlet related to the mapping is not enabled, skip it from consideration + if (!servletHolder.isEnabled()) + continue; + + //only accept a default mapping if we don't have any other + if (finalMapping == null) + finalMapping = mapping; else { - //existing candidate isn't a default, if the one we're looking at isn't a default either, then its an error - if (!mapping.isFromDefaultDescriptor()) + //already have a candidate - only accept another one + //if the candidate is a default, or we're allowing duplicate mappings + if (finalMapping.isFromDefaultDescriptor()) + finalMapping = mapping; + else if (isAllowDuplicateMappings()) { - ServletHolder finalMappedServlet = getServlet(finalMapping.getServletName()); - throw new IllegalStateException("Multiple servlets map to path " + - pathSpec + ": " + - finalMappedServlet.getName() + "[mapped:" + finalMapping.getSource() + "]," + - mapping.getServletName() + "[mapped:" + mapping.getSource() + "]"); + LOG.warn("Multiple servlets map to path {}: {} and {}, choosing {}", pathSpec, finalMapping.getServletName(), mapping.getServletName(), mapping); + finalMapping = mapping; + } + else + { + //existing candidate isn't a default, if the one we're looking at isn't a default either, then its an error + if (!mapping.isFromDefaultDescriptor()) + { + ServletHolder finalMappedServlet = getServlet(finalMapping.getServletName()); + throw new IllegalStateException("Multiple servlets map to path " + + pathSpec + ": " + + finalMappedServlet.getName() + "[mapped:" + finalMapping.getSource() + "]," + + mapping.getServletName() + "[mapped:" + mapping.getSource() + "]"); + } } } } + if (finalMapping == null) + throw new IllegalStateException("No acceptable servlet mappings for " + pathSpec); + + if (LOG.isDebugEnabled()) + LOG.debug("Path={}[{}] mapped to servlet={}[{}]", + pathSpec, + finalMapping.getSource(), + finalMapping.getServletName(), + getServlet(finalMapping.getServletName()).getSource()); + + ServletPathSpec servletPathSpec = new ServletPathSpec(pathSpec); + MappedServlet mappedServlet = new MappedServlet(servletPathSpec, getServlet(finalMapping.getServletName())); + pm.put(servletPathSpec, mappedServlet); } - if (finalMapping == null) - throw new IllegalStateException("No acceptable servlet mappings for " + pathSpec); - if (LOG.isDebugEnabled()) - LOG.debug("Path={}[{}] mapped to servlet={}[{}]", - pathSpec, - finalMapping.getSource(), - finalMapping.getServletName(), - getServlet(finalMapping.getServletName()).getSource()); - - ServletPathSpec servletPathSpec = new ServletPathSpec(pathSpec); - MappedServlet mappedServlet = new MappedServlet(servletPathSpec, getServlet(finalMapping.getServletName())); - pm.put(servletPathSpec, mappedServlet); + _servletPathMap = pm; } - _servletPathMap = pm; - } + // flush filter chain cache + for (int i = _chainCache.length; i-- > 0; ) + { + if (_chainCache[i] != null) + _chainCache[i].clear(); + } - // flush filter chain cache - for (int i = _chainCache.length; i-- > 0; ) - { - if (_chainCache[i] != null) - _chainCache[i].clear(); - } + if (LOG.isDebugEnabled()) + { + LOG.debug("filterNameMap=" + _filterNameMap); + LOG.debug("pathFilters=" + _filterPathMappings); + LOG.debug("servletFilterMap=" + _filterNameMappings); + LOG.debug("servletPathMap=" + _servletPathMap); + LOG.debug("servletNameMap=" + _servletNameMap); + } - if (LOG.isDebugEnabled()) - { - LOG.debug("filterNameMap=" + _filterNameMap); - LOG.debug("pathFilters=" + _filterPathMappings); - LOG.debug("servletFilterMap=" + _filterNameMappings); - LOG.debug("servletPathMap=" + _servletPathMap); - LOG.debug("servletNameMap=" + _servletNameMap); - } - - try - { - if (_contextHandler != null && _contextHandler.isStarted() || _contextHandler == null && isStarted()) - initialize(); - } - catch (Exception e) - { - throw new RuntimeException(e); + try + { + if (_contextHandler != null && _contextHandler.isStarted() || _contextHandler == null && isStarted()) + initialize(); + } + catch (Exception e) + { + throw new RuntimeException(e); + } } } @@ -1426,30 +1425,36 @@ public class ServletHandler extends ScopedHandler nextHandle(baseRequest.getPathInContext(), baseRequest, request, response); } - protected synchronized boolean containsFilterHolder(FilterHolder holder) + protected boolean containsFilterHolder(FilterHolder holder) { - if (_filters == null) - return false; - for (FilterHolder f : _filters) + try (AutoLock ignored = lock()) { - if (f == holder) - return true; + if (_filters == null) + return false; + for (FilterHolder f : _filters) + { + if (f == holder) + return true; + } + return false; } - return false; } - protected synchronized boolean containsServletHolder(ServletHolder holder) + protected boolean containsServletHolder(ServletHolder holder) { - if (_servlets == null) - return false; - for (ServletHolder s : _servlets) + try (AutoLock ignored = lock()) { - @SuppressWarnings("ReferenceEquality") - boolean foundServletHolder = (s == holder); - if (foundServletHolder) - return true; + if (_servlets == null) + return false; + for (ServletHolder s : _servlets) + { + @SuppressWarnings("ReferenceEquality") + boolean foundServletHolder = (s == holder); + if (foundServletHolder) + return true; + } + return false; } - return false; } /** @@ -1472,17 +1477,22 @@ public class ServletHandler extends ScopedHandler invalidateChainsCache(); } - public synchronized void setFilters(FilterHolder[] holders) + public void setFilters(FilterHolder[] holders) { - if (holders != null) - for (FilterHolder holder : holders) + try (AutoLock ignored = lock()) + { + if (holders != null) { - holder.setServletHandler(this); + for (FilterHolder holder : holders) + { + holder.setServletHandler(this); + } } - updateBeans(_filters,holders); - _filters = holders; - updateNameMappings(); - invalidateChainsCache(); + updateBeans(_filters,holders); + _filters = holders; + updateNameMappings(); + invalidateChainsCache(); + } } /** @@ -1502,17 +1512,22 @@ public class ServletHandler extends ScopedHandler * * @param holders Array of servlets to define */ - public synchronized void setServlets(ServletHolder[] holders) + public void setServlets(ServletHolder[] holders) { - if (holders != null) - for (ServletHolder holder : holders) + try (AutoLock ignored = lock()) + { + if (holders != null) { - holder.setServletHandler(this); + for (ServletHolder holder : holders) + { + holder.setServletHandler(this); + } } - updateBeans(_servlets,holders); - _servlets = holders; - updateNameMappings(); - invalidateChainsCache(); + updateBeans(_servlets,holders); + _servlets = holders; + updateNameMappings(); + invalidateChainsCache(); + } } protected class CachedChain implements FilterChain diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java index 4e42bb6f014..10e826058fa 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHolder.java @@ -58,6 +58,7 @@ 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.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -172,11 +173,10 @@ public class ServletHolder extends Holder implements UserIdentity.Scope return null; } - public synchronized void setServlet(Servlet servlet) + public void setServlet(Servlet servlet) { if (servlet == null || servlet instanceof SingleThreadModel) throw new IllegalArgumentException(SingleThreadModel.class.getName() + " has been deprecated since Servlet API 2.4"); - setInstance(servlet); } @@ -254,11 +254,14 @@ public class ServletHolder extends Holder implements UserIdentity.Scope * @param name The role name as used by the servlet * @param link The role name as used by the container. */ - public synchronized void setUserRoleLink(String name, String link) + public void setUserRoleLink(String name, String link) { - if (_roleMap == null) - _roleMap = new HashMap<>(); - _roleMap.put(name, link); + try (AutoLock ignored = lock()) + { + if (_roleMap == null) + _roleMap = new HashMap<>(); + _roleMap.put(name, link); + } } /** @@ -270,10 +273,13 @@ public class ServletHolder extends Holder implements UserIdentity.Scope */ public String getUserRoleLink(String name) { - if (_roleMap == null) - return name; - String link = _roleMap.get(name); - return (link == null) ? name : link; + try (AutoLock ignored = lock()) + { + if (_roleMap == null) + return name; + String link = _roleMap.get(name); + return (link == null) ? name : link; + } } /** @@ -393,7 +399,7 @@ public class ServletHolder extends Holder implements UserIdentity.Scope _config = new Config(); - synchronized (this) + try (AutoLock ignored = lock()) { if (getHeldClass() != null && javax.servlet.SingleThreadModel.class.isAssignableFrom(getHeldClass())) _servlet = new SingleThreadedWrapper(); @@ -404,7 +410,7 @@ public class ServletHolder extends Holder implements UserIdentity.Scope public void initialize() throws Exception { - synchronized (this) + try (AutoLock ignored = lock()) { if (_servlet == null && (_initOnStartup || isInstance())) { @@ -418,7 +424,7 @@ public class ServletHolder extends Holder implements UserIdentity.Scope public void doStop() throws Exception { - synchronized (this) + try (AutoLock ignored = lock()) { Servlet servlet = _servlet; if (servlet != null) @@ -464,7 +470,7 @@ public class ServletHolder extends Holder implements UserIdentity.Scope public Servlet getServlet() throws ServletException { - synchronized (this) + try (AutoLock ignored = lock()) { if (_servlet == null && isRunning()) { @@ -525,7 +531,7 @@ public class ServletHolder extends Holder implements UserIdentity.Scope private Servlet makeUnavailable(UnavailableException e) { - synchronized (this) + try (AutoLock ignored = lock()) { _servlet = new UnavailableServlet(e, _servlet); return _servlet; @@ -553,10 +559,10 @@ public class ServletHolder extends Holder implements UserIdentity.Scope } } - private synchronized void initServlet() + private void initServlet() throws ServletException { - try + try (AutoLock ignored = lock()) { if (_servlet == null) _servlet = getInstance(); @@ -1010,12 +1016,12 @@ public class ServletHolder extends Holder implements UserIdentity.Scope private class SingleThreadedWrapper implements Servlet { - Stack _stack = new Stack<>(); + private final Stack _stack = new Stack<>(); @Override public void destroy() { - synchronized (this) + try (AutoLock ignored = lock()) { while (_stack.size() > 0) { @@ -1047,7 +1053,7 @@ public class ServletHolder extends Holder implements UserIdentity.Scope @Override public void init(ServletConfig config) throws ServletException { - synchronized (this) + try (AutoLock ignored = lock()) { if (_stack.size() == 0) { @@ -1073,10 +1079,10 @@ public class ServletHolder extends Holder implements UserIdentity.Scope public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException { Servlet s; - synchronized (this) + try (AutoLock ignored = lock()) { if (_stack.size() > 0) - s = (Servlet)_stack.pop(); + s = _stack.pop(); else { try @@ -1101,7 +1107,7 @@ public class ServletHolder extends Holder implements UserIdentity.Scope } finally { - synchronized (this) + try (AutoLock ignored = lock()) { _stack.push(s); } @@ -1123,16 +1129,19 @@ public class ServletHolder extends Holder implements UserIdentity.Scope } @Override - protected synchronized Servlet createInstance() throws Exception + protected Servlet createInstance() throws Exception { - Servlet servlet = super.createInstance(); - if (servlet == null) + try (AutoLock ignored = lock()) { - ServletContext ctx = getServletContext(); - if (ctx != null) - servlet = ctx.createServlet(getHeldClass()); + Servlet servlet = super.createInstance(); + if (servlet == null) + { + ServletContext ctx = getServletContext(); + if (ctx != null) + servlet = ctx.createServlet(getHeldClass()); + } + return servlet; } - return servlet; } @Override @@ -1196,7 +1205,7 @@ public class ServletHolder extends Holder implements UserIdentity.Scope ((HttpServletResponse)res).sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE); else { - synchronized (ServletHolder.this) + try (AutoLock ignored = lock()) { ServletHolder.this._servlet = this._servlet; _servlet.service(req, res); diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java index ab4b85a0869..ab11987faaf 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/DoSFilter.java @@ -57,6 +57,7 @@ import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; import org.eclipse.jetty.util.annotation.Name; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; import org.eclipse.jetty.util.thread.Scheduler; import org.slf4j.Logger; @@ -1127,6 +1128,7 @@ public class DoSFilter implements Filter { private static final long serialVersionUID = 3534663738034577872L; + final AutoLock _lock = new AutoLock(); protected final String _filterName; protected transient ServletContext _context; protected final String _id; @@ -1152,7 +1154,7 @@ public class DoSFilter implements Filter public boolean isRateExceeded(long now) { final long last; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { last = _timestamps[_next]; _timestamps[_next] = now; @@ -1281,7 +1283,7 @@ public class DoSFilter implements Filter // rate limit is never exceeded, but we keep track of the request timestamps // so that we know whether there was recent activity on this tracker // and whether it should be expired - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _timestamps[_next] = now; _next = (_next + 1) % _timestamps.length; diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/EventSourceServlet.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/EventSourceServlet.java index 4506dbf592e..376c7d5eeb3 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/EventSourceServlet.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/EventSourceServlet.java @@ -34,6 +34,8 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.util.thread.AutoLock; + /** *

A servlet that implements the event source protocol, * also known as "server sent events".

@@ -42,11 +44,11 @@ import javax.servlet.http.HttpServletResponse; * and to emit event source events.

*

This servlet supports the following configuration parameters:

*
    - *
  • heartBeatPeriod, that specifies the heartbeat period, in seconds, used to check + *
  • {@code heartBeatPeriod}, that specifies the heartbeat period, in seconds, used to check * whether the connection has been closed by the client; defaults to 10 seconds.
    • *
* - *

NOTE: there is currently no support for last-event-id.

+ *

NOTE: there is currently no support for {@code last-event-id}.

*/ public abstract class EventSourceServlet extends HttpServlet { @@ -77,7 +79,6 @@ public abstract class EventSourceServlet extends HttpServlet @Override protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - @SuppressWarnings("unchecked") Enumeration acceptValues = request.getHeaders("Accept"); while (acceptValues.hasMoreElements()) { @@ -127,6 +128,7 @@ public abstract class EventSourceServlet extends HttpServlet protected class EventSourceEmitter implements EventSource.Emitter, Runnable { + private final AutoLock lock = new AutoLock(); private final EventSource eventSource; private final AsyncContext async; private final ServletOutputStream output; @@ -143,7 +145,7 @@ public abstract class EventSourceServlet extends HttpServlet @Override public void event(String name, String data) throws IOException { - synchronized (this) + try (AutoLock ignored = lock.lock()) { output.write(EVENT_FIELD); output.write(name.getBytes(StandardCharsets.UTF_8)); @@ -155,7 +157,7 @@ public abstract class EventSourceServlet extends HttpServlet @Override public void data(String data) throws IOException { - synchronized (this) + try (AutoLock ignored = lock.lock()) { BufferedReader reader = new BufferedReader(new StringReader(data)); String line; @@ -173,7 +175,7 @@ public abstract class EventSourceServlet extends HttpServlet @Override public void comment(String comment) throws IOException { - synchronized (this) + try (AutoLock ignored = lock.lock()) { output.write(COMMENT_FIELD); output.write(comment.getBytes(StandardCharsets.UTF_8)); @@ -191,7 +193,7 @@ public abstract class EventSourceServlet extends HttpServlet // on the second flush() try { - synchronized (this) + try (AutoLock ignored = lock.lock()) { output.write('\r'); flush(); @@ -217,7 +219,7 @@ public abstract class EventSourceServlet extends HttpServlet @Override public void close() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { closed = true; heartBeat.cancel(false); @@ -227,7 +229,7 @@ public abstract class EventSourceServlet extends HttpServlet private void scheduleHeartBeat() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (!closed) heartBeat = scheduler.schedule(this, heartBeatPeriod, TimeUnit.SECONDS); diff --git a/jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONDateConvertor.java b/jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONDateConvertor.java index 02540fefa81..a3d63a0941d 100644 --- a/jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONDateConvertor.java +++ b/jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONDateConvertor.java @@ -27,6 +27,7 @@ import java.util.TimeZone; import org.eclipse.jetty.util.DateCache; import org.eclipse.jetty.util.ajax.JSON.Output; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,6 +41,7 @@ public class JSONDateConvertor implements JSON.Convertor { private static final Logger LOG = LoggerFactory.getLogger(JSONDateConvertor.class); + private final AutoLock _lock = new AutoLock(); private final boolean _fromJSON; private final DateCache _dateCache; private final SimpleDateFormat _format; @@ -77,7 +79,7 @@ public class JSONDateConvertor implements JSON.Convertor throw new UnsupportedOperationException(); try { - synchronized (_format) + try (AutoLock ignored = _lock.lock()) { return _format.parseObject((String)map.get("value")); } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/RolloverFileOutputStream.java b/jetty-util/src/main/java/org/eclipse/jetty/util/RolloverFileOutputStream.java index 25cb183c892..c91fd47915b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/RolloverFileOutputStream.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/RolloverFileOutputStream.java @@ -32,6 +32,8 @@ import java.util.TimeZone; import java.util.Timer; import java.util.TimerTask; +import org.eclipse.jetty.util.thread.AutoLock; + /** * RolloverFileOutputStream. * @@ -53,11 +55,11 @@ public class RolloverFileOutputStream extends OutputStream static final String ROLLOVER_FILE_BACKUP_FORMAT = "HHmmssSSS"; static final int ROLLOVER_FILE_RETAIN_DAYS = 31; + private final AutoLock _lock = new AutoLock(); private OutputStream _out; private RollTask _rollTask; private SimpleDateFormat _fileBackupFormat; private SimpleDateFormat _fileDateFormat; - private String _filename; private File _file; private boolean _append; @@ -176,11 +178,7 @@ public class RolloverFileOutputStream extends OutputStream // Calculate Today's Midnight, based on Configured TimeZone (will be in past, even if by a few milliseconds) setFile(now); - synchronized (RolloverFileOutputStream.class) - { - if (__rollover == null) - __rollover = new Timer(RolloverFileOutputStream.class.getName(), true); - } + __rollover = new Timer(RolloverFileOutputStream.class.getName(), true); // This will schedule the rollover event to the next midnight scheduleNextRollover(now); @@ -205,10 +203,7 @@ public class RolloverFileOutputStream extends OutputStream // Schedule next rollover event to occur, based on local machine's Unix Epoch milliseconds long delay = midnight.toInstant().toEpochMilli() - now.toInstant().toEpochMilli(); - synchronized (RolloverFileOutputStream.class) - { - __rollover.schedule(_rollTask, delay); - } + __rollover.schedule(_rollTask, delay); } public String getFilename() @@ -234,7 +229,7 @@ public class RolloverFileOutputStream extends OutputStream File oldFile = null; File newFile = null; File backupFile = null; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { // Check directory File file = new File(_filename); @@ -350,27 +345,25 @@ public class RolloverFileOutputStream extends OutputStream @Override public void write(int b) throws IOException { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _out.write(b); } } @Override - public void write(byte[] buf) - throws IOException + public void write(byte[] buf) throws IOException { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _out.write(buf); } } @Override - public void write(byte[] buf, int off, int len) - throws IOException + public void write(byte[] buf, int off, int len) throws IOException { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _out.write(buf, off, len); } @@ -379,17 +372,16 @@ public class RolloverFileOutputStream extends OutputStream @Override public void flush() throws IOException { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _out.flush(); } } @Override - public void close() - throws IOException + public void close() throws IOException { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { try { @@ -402,13 +394,9 @@ public class RolloverFileOutputStream extends OutputStream } } - synchronized (RolloverFileOutputStream.class) - { - if (_rollTask != null) - { - _rollTask.cancel(); - } - } + RollTask rollTask = _rollTask; + if (rollTask != null) + rollTask.cancel(); } private class RollTask extends TimerTask diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java index 2ac15ca7b2c..9a28572269e 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Scanner.java @@ -43,6 +43,7 @@ import java.util.TimerTask; import java.util.function.Predicate; import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -63,16 +64,17 @@ public class Scanner extends AbstractLifeCycle */ public static final int DEFAULT_SCAN_DEPTH = 1; public static final int MAX_SCAN_DEPTH = Integer.MAX_VALUE; - private static final Logger LOG = LoggerFactory.getLogger(Scanner.class); private static int __scannerId = 0; - private int _scanInterval; - private int _scanCount = 0; + + private final AutoLock _lock = new AutoLock(); private final List _listeners = new ArrayList<>(); private final Map _prevScan = new HashMap<>(); private final Map _currentScan = new HashMap<>(); - private FilenameFilter _filter; private final Map> _scannables = new HashMap<>(); + private int _scanInterval; + private int _scanCount = 0; + private FilenameFilter _filter; private volatile boolean _running = false; private boolean _reportExisting = true; private boolean _reportDirs = true; @@ -296,9 +298,9 @@ public class Scanner extends AbstractLifeCycle * * @return interval between scans in seconds */ - public synchronized int getScanInterval() + public int getScanInterval() { - return _scanInterval; + return _lock.runLocked(() -> _scanInterval); } /** @@ -306,10 +308,13 @@ public class Scanner extends AbstractLifeCycle * * @param scanInterval pause between scans in seconds, or 0 for no scan after the initial scan. */ - public synchronized void setScanInterval(int scanInterval) + public void setScanInterval(int scanInterval) { - _scanInterval = scanInterval; - schedule(); + try (AutoLock ignored = _lock.lock()) + { + _scanInterval = scanInterval; + schedule(); + } } public void setScanDirs(List dirs) @@ -325,11 +330,11 @@ public class Scanner extends AbstractLifeCycle } @Deprecated - public synchronized void addScanDir(File dir) + public void addScanDir(File dir) { if (dir == null) return; - try + try (AutoLock ignored = _lock.lock()) { if (dir.isDirectory()) addDirectory(dir.toPath()); @@ -348,15 +353,14 @@ public class Scanner extends AbstractLifeCycle * @param p the Path of the file to scan. * @throws IOException */ - public synchronized void addFile(Path p) throws IOException + public void addFile(Path p) throws IOException { if (p == null) throw new IllegalStateException("Null path"); - File f = p.toFile(); if (!f.exists() || f.isDirectory()) throw new IllegalStateException("Not file or doesn't exist: " + f.getCanonicalPath()); - _scannables.put(p, null); + _lock.runLocked(() -> _scannables.put(p, null)); } /** @@ -366,24 +370,24 @@ public class Scanner extends AbstractLifeCycle * @return an IncludeExcludeSet to which the caller can add PathMatcher patterns to match * @throws IOException */ - public synchronized IncludeExcludeSet addDirectory(Path p) - throws IOException + public IncludeExcludeSet addDirectory(Path p) throws IOException { if (p == null) throw new IllegalStateException("Null path"); - File f = p.toFile(); if (!f.exists() || !f.isDirectory()) throw new IllegalStateException("Not directory or doesn't exist: " + f.getCanonicalPath()); - - IncludeExcludeSet includesExcludes = _scannables.get(p); - if (includesExcludes == null) + + try (AutoLock ignored = _lock.lock()) { - includesExcludes = new IncludeExcludeSet<>(PathMatcherSet.class); - _scannables.put(p.toRealPath(), includesExcludes); + IncludeExcludeSet includesExcludes = _scannables.get(p); + if (includesExcludes == null) + { + includesExcludes = new IncludeExcludeSet<>(PathMatcherSet.class); + _scannables.put(p.toRealPath(), includesExcludes); + } + return includesExcludes; } - - return includesExcludes; } @Deprecated @@ -500,11 +504,11 @@ public class Scanner extends AbstractLifeCycle * * @param listener the listener to add */ - public synchronized void addListener(Listener listener) + public void addListener(Listener listener) { if (listener == null) return; - _listeners.add(listener); + _lock.runLocked(() -> _listeners.add(listener)); } /** @@ -512,40 +516,43 @@ public class Scanner extends AbstractLifeCycle * * @param listener the Listener to be removed */ - public synchronized void removeListener(Listener listener) + public void removeListener(Listener listener) { if (listener == null) return; - _listeners.remove(listener); + _lock.runLocked(() -> _listeners.remove(listener)); } /** * Start the scanning action. */ @Override - public synchronized void doStart() + public void doStart() { - if (_running) - return; - - _running = true; - if (LOG.isDebugEnabled()) - LOG.debug("Scanner start: rprtExists={}, depth={}, rprtDirs={}, interval={}, filter={}, scannables={}", - _reportExisting, _scanDepth, _reportDirs, _scanInterval, _filter, _scannables); - - if (_reportExisting) + try (AutoLock ignored = _lock.lock()) { - // if files exist at startup, report them - scan(); - scan(); // scan twice so files reported as stable + if (_running) + return; + + _running = true; + if (LOG.isDebugEnabled()) + LOG.debug("Scanner start: rprtExists={}, depth={}, rprtDirs={}, interval={}, filter={}, scannables={}", + _reportExisting, _scanDepth, _reportDirs, _scanInterval, _filter, _scannables); + + if (_reportExisting) + { + // if files exist at startup, report them + scan(); + scan(); // scan twice so files reported as stable + } + else + { + //just register the list of existing files and only report changes + scanFiles(); + _prevScan.putAll(_currentScan); + } + schedule(); } - else - { - //just register the list of existing files and only report changes - scanFiles(); - _prevScan.putAll(_currentScan); - } - schedule(); } public TimerTask newTimerTask() @@ -586,17 +593,20 @@ public class Scanner extends AbstractLifeCycle * Stop the scanning. */ @Override - public synchronized void doStop() + public void doStop() { - if (_running) + try (AutoLock ignored = _lock.lock()) { - _running = false; - if (_timer != null) - _timer.cancel(); - if (_task != null) - _task.cancel(); - _task = null; - _timer = null; + if (_running) + { + _running = false; + if (_timer != null) + _timer.cancel(); + if (_task != null) + _task.cancel(); + _task = null; + _timer = null; + } } } @@ -634,25 +644,28 @@ public class Scanner extends AbstractLifeCycle /** * Perform a pass of the scanner and report changes */ - public synchronized void scan() + public void scan() { - reportScanStart(++_scanCount); - scanFiles(); - reportDifferences(_currentScan, _prevScan); - _prevScan.clear(); - _prevScan.putAll(_currentScan); - reportScanEnd(_scanCount); - - for (Listener l : _listeners) + try (AutoLock ignored = _lock.lock()) { - try + reportScanStart(++_scanCount); + scanFiles(); + reportDifferences(_currentScan, _prevScan); + _prevScan.clear(); + _prevScan.putAll(_currentScan); + reportScanEnd(_scanCount); + + for (Listener l : _listeners) { - if (l instanceof ScanListener) - ((ScanListener)l).scan(); - } - catch (Throwable e) - { - LOG.warn("Unable to scan", e); + try + { + if (l instanceof ScanListener) + ((ScanListener)l).scan(); + } + catch (Throwable e) + { + LOG.warn("Unable to scan", e); + } } } } @@ -660,18 +673,21 @@ public class Scanner extends AbstractLifeCycle /** * Scan all of the given paths. */ - public synchronized void scanFiles() + public void scanFiles() { - _currentScan.clear(); - for (Path p : _scannables.keySet()) + try (AutoLock ignored = _lock.lock()) { - try + _currentScan.clear(); + for (Path p : _scannables.keySet()) { - Files.walkFileTree(p, EnumSet.allOf(FileVisitOption.class),_scanDepth, new Visitor(p, _scannables.get(p), _currentScan)); - } - catch (IOException e) - { - LOG.warn("Error scanning files.", e); + try + { + Files.walkFileTree(p, EnumSet.allOf(FileVisitOption.class),_scanDepth, new Visitor(p, _scannables.get(p), _currentScan)); + } + catch (IOException e) + { + LOG.warn("Error scanning files.", e); + } } } } @@ -682,91 +698,94 @@ public class Scanner extends AbstractLifeCycle * @param currentScan the info from the most recent pass * @param oldScan info from the previous pass */ - private synchronized void reportDifferences(Map currentScan, Map oldScan) + private void reportDifferences(Map currentScan, Map oldScan) { - // scan the differences and add what was found to the map of notifications: - Set oldScanKeys = new HashSet<>(oldScan.keySet()); - - // Look for new and changed files - for (Map.Entry entry : currentScan.entrySet()) + try (AutoLock ignored = _lock.lock()) { - String file = entry.getKey(); - if (!oldScanKeys.contains(file)) + // scan the differences and add what was found to the map of notifications: + Set oldScanKeys = new HashSet<>(oldScan.keySet()); + + // Look for new and changed files + for (Entry entry : currentScan.entrySet()) { - Notification old = _notifications.put(file, Notification.ADDED); - if (old != null) + String file = entry.getKey(); + if (!oldScanKeys.contains(file)) { - switch (old) + Notification old = _notifications.put(file, Notification.ADDED); + if (old != null) { - case REMOVED: - case CHANGED: - _notifications.put(file, Notification.CHANGED); - break; - default: - break; + switch (old) + { + case REMOVED: + case CHANGED: + _notifications.put(file, Notification.CHANGED); + break; + default: + break; + } } } + else if (!oldScan.get(file).equals(currentScan.get(file))) + { + Notification old = _notifications.put(file, Notification.CHANGED); + if (old == Notification.ADDED) + _notifications.put(file, Notification.ADDED); + } } - else if (!oldScan.get(file).equals(currentScan.get(file))) + + // Look for deleted files + for (String file : oldScan.keySet()) { - Notification old = _notifications.put(file, Notification.CHANGED); - if (old == Notification.ADDED) - _notifications.put(file, Notification.ADDED); + if (!currentScan.containsKey(file)) + { + Notification old = _notifications.put(file, Notification.REMOVED); + if (old == Notification.ADDED) + _notifications.remove(file); + } } - } - // Look for deleted files - for (String file : oldScan.keySet()) - { - if (!currentScan.containsKey(file)) + if (LOG.isDebugEnabled()) + LOG.debug("scanned " + _scannables.keySet() + ": " + _notifications); + + // Process notifications + // Only process notifications that are for stable files (ie same in old and current scan). + List bulkChanges = new ArrayList<>(); + for (Iterator> iter = _notifications.entrySet().iterator(); iter.hasNext(); ) { - Notification old = _notifications.put(file, Notification.REMOVED); - if (old == Notification.ADDED) - _notifications.remove(file); - } - } - if (LOG.isDebugEnabled()) - LOG.debug("scanned " + _scannables.keySet() + ": " + _notifications); - - // Process notifications - // Only process notifications that are for stable files (ie same in old and current scan). - List bulkChanges = new ArrayList<>(); - for (Iterator> iter = _notifications.entrySet().iterator(); iter.hasNext(); ) - { - - Entry entry = iter.next(); - String file = entry.getKey(); - // Is the file stable? - if (oldScan.containsKey(file)) - { - if (!oldScan.get(file).equals(currentScan.get(file))) + Entry entry = iter.next(); + String file = entry.getKey(); + // Is the file stable? + if (oldScan.containsKey(file)) + { + if (!oldScan.get(file).equals(currentScan.get(file))) + continue; + } + else if (currentScan.containsKey(file)) continue; - } - else if (currentScan.containsKey(file)) - continue; - // File is stable so notify - Notification notification = entry.getValue(); - iter.remove(); - bulkChanges.add(file); - switch (notification) - { - case ADDED: - reportAddition(file); - break; - case CHANGED: - reportChange(file); - break; - case REMOVED: - reportRemoval(file); - break; - default: - break; + // File is stable so notify + Notification notification = entry.getValue(); + iter.remove(); + bulkChanges.add(file); + switch (notification) + { + case ADDED: + reportAddition(file); + break; + case CHANGED: + reportChange(file); + break; + case REMOVED: + reportRemoval(file); + break; + default: + break; + } } + if (!bulkChanges.isEmpty()) + reportBulkChanges(bulkChanges); } - if (!bulkChanges.isEmpty()) - reportBulkChanges(bulkChanges); } private void warn(Object listener, String filename, Throwable th) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java index 77aebf33779..9b08454e42f 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/StringUtil.java @@ -376,15 +376,12 @@ public class StringUtil int offset, int length) { - synchronized (buf) + int end = offset + length; + for (int i = offset; i < end; i++) { - int end = offset + length; - for (int i = offset; i < end; i++) - { - if (i >= s.length()) - break; - buf.append(s.charAt(i)); - } + if (i >= s.length()) + break; + buf.append(s.charAt(i)); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java index c7c88b09b67..fa381aa9dbc 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java @@ -1180,27 +1180,24 @@ public class URIUtil */ public static void appendSchemeHostPort(StringBuffer url, String scheme, String server, int port) { - synchronized (url) + url.append(scheme).append("://").append(HostPort.normalizeHost(server)); + + if (port > 0) { - url.append(scheme).append("://").append(HostPort.normalizeHost(server)); - - if (port > 0) + switch (scheme) { - switch (scheme) - { - case "http": - if (port != 80) - url.append(':').append(port); - break; - - case "https": - if (port != 443) - url.append(':').append(port); - break; - - default: + case "http": + if (port != 80) url.append(':').append(port); - } + break; + + case "https": + if (port != 443) + url.append(':').append(port); + break; + + default: + url.append(':').append(port); } } } 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..906d72ee8f3 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 @@ -140,7 +140,7 @@ public class UrlEncoded extends MultiMap implements Cloneable * 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) + public String encode(Charset charset, boolean equalsForNullValue) { return encode(this, charset, equalsForNullValue); } @@ -238,64 +238,61 @@ 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, ""); + } + 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 '+': + encoded = true; + break; + 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); + } + 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, ""); } } } @@ -316,76 +313,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 = null; + + 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; + value = 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 +394,71 @@ 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 = null; + + 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; + value = 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 +473,71 @@ 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 = null; + + 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; + value = 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 @@ -627,77 +615,74 @@ public class UrlEncoded extends MultiMap implements Cloneable return; } - synchronized (map) + String key = null; + String value = null; + + int c; + + int totalLength = 0; + + try (ByteArrayOutputStream2 output = new ByteArrayOutputStream2()) { - String key = null; - String value = null; + int size = 0; - 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; + value = 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); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java index 8925683b128..ccc7d68c48b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AbstractLifeCycle.java @@ -26,6 +26,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.util.Uptime; import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,7 +54,7 @@ public abstract class AbstractLifeCycle implements LifeCycle public static final String STOPPING = State.STOPPING.toString(); private final List _eventListener = new CopyOnWriteArrayList<>(); - private final Object _lock = new Object(); + private final AutoLock _lock = new AutoLock(); private volatile State _state = State.STOPPED; /** @@ -76,7 +77,7 @@ public abstract class AbstractLifeCycle implements LifeCycle @Override public final void start() throws Exception { - synchronized (_lock) + try (AutoLock ignored = _lock.lock()) { try { @@ -117,7 +118,7 @@ public abstract class AbstractLifeCycle implements LifeCycle @Override public final void stop() throws Exception { - synchronized (_lock) + try (AutoLock ignored = _lock.lock()) { try { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AttributeContainerMap.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AttributeContainerMap.java index 22749d30972..f6165e9bc32 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/AttributeContainerMap.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/AttributeContainerMap.java @@ -26,52 +26,63 @@ import java.util.Map; import java.util.Set; import org.eclipse.jetty.util.Attributes; +import org.eclipse.jetty.util.thread.AutoLock; /** * An Attributes implementation that holds it's values in an immutable {@link ContainerLifeCycle} */ public class AttributeContainerMap extends ContainerLifeCycle implements Attributes { + private final AutoLock _lock = new AutoLock(); private final Map _map = new HashMap<>(); @Override - public synchronized void setAttribute(String name, Object attribute) + public void setAttribute(String name, Object attribute) { - Object old = _map.put(name, attribute); - updateBean(old, attribute); + try (AutoLock ignored = _lock.lock()) + { + Object old = _map.put(name, attribute); + updateBean(old, attribute); + } } @Override - public synchronized void removeAttribute(String name) + public void removeAttribute(String name) { - Object removed = _map.remove(name); - if (removed != null) - removeBean(removed); + try (AutoLock ignored = _lock.lock()) + { + Object removed = _map.remove(name); + if (removed != null) + removeBean(removed); + } } @Override - public synchronized Object getAttribute(String name) + public Object getAttribute(String name) { - return _map.get(name); + return _lock.runLocked(() -> _map.get(name)); } @Override - public synchronized Enumeration getAttributeNames() + public Enumeration getAttributeNames() { - return Collections.enumeration(_map.keySet()); + return _lock.runLocked(() -> Collections.enumeration(getAttributeNameSet())); } @Override public Set getAttributeNameSet() { - return _map.keySet(); + return _lock.runLocked(_map::keySet); } @Override - public synchronized void clearAttributes() + public void clearAttributes() { - _map.clear(); - this.removeBeans(); + try (AutoLock ignored = _lock.lock()) + { + _map.clear(); + this.removeBeans(); + } } @Override diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java index ef488981ff8..c1849466820 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarFileResource.java @@ -30,12 +30,14 @@ import java.util.jar.JarEntry; import java.util.jar.JarFile; import org.eclipse.jetty.util.URIUtil; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class JarFileResource extends JarResource { private static final Logger LOG = LoggerFactory.getLogger(JarFileResource.class); + private JarFile _jarFile; private File _file; private String[] _list; @@ -51,73 +53,81 @@ public class JarFileResource extends JarResource } @Override - public synchronized void close() + public void close() { - _exists = false; - _list = null; - _entry = null; - _file = null; - //if the jvm is not doing url caching, then the JarFiles will not be cached either, - //and so they are safe to close - if (!getUseCaches()) + try (AutoLock ignored = _lock.lock()) { - if (_jarFile != null) + _exists = false; + _list = null; + _entry = null; + _file = null; + //if the jvm is not doing url caching, then the JarFiles will not be cached either, + //and so they are safe to close + if (!getUseCaches()) { - try + if (_jarFile != null) { - if (LOG.isDebugEnabled()) - LOG.debug("Closing JarFile " + _jarFile.getName()); - _jarFile.close(); - } - catch (IOException ioe) - { - LOG.trace("IGNORED", ioe); + try + { + if (LOG.isDebugEnabled()) + LOG.debug("Closing JarFile " + _jarFile.getName()); + _jarFile.close(); + } + catch (IOException ioe) + { + LOG.trace("IGNORED", ioe); + } } } + _jarFile = null; + super.close(); } - _jarFile = null; - super.close(); } @Override - protected synchronized boolean checkConnection() + protected boolean checkConnection() { - try + try (AutoLock ignored = _lock.lock()) { - super.checkConnection(); - } - finally - { - if (_jarConnection == null) + try { - _entry = null; - _file = null; - _jarFile = null; - _list = null; + super.checkConnection(); } + finally + { + if (_jarConnection == null) + { + _entry = null; + _file = null; + _jarFile = null; + _list = null; + } + } + return _jarFile != null; } - return _jarFile != null; } @Override - protected synchronized void newConnection() - throws IOException + protected void newConnection() throws IOException { - super.newConnection(); + try (AutoLock ignored = _lock.lock()) + { + super.newConnection(); - _entry = null; - _file = null; - _jarFile = null; - _list = null; + _entry = null; + _file = null; + _jarFile = null; + _list = null; - // Work with encoded URL path (_urlString is assumed to be encoded) - int sep = _urlString.lastIndexOf("!/"); - _jarUrl = _urlString.substring(0, sep + 2); - _path = URIUtil.decodePath(_urlString.substring(sep + 2)); - if (_path.length() == 0) - _path = null; - _jarFile = _jarConnection.getJarFile(); - _file = new File(_jarFile.getName()); + // Work with encoded URL path (_urlString is assumed to be encoded) + int sep = _urlString.lastIndexOf("!/"); + _jarUrl = _urlString.substring(0, sep + 2); + _path = URIUtil.decodePath(_urlString.substring(sep + 2)); + if (_path.length() == 0) + _path = null; + _jarFile = _jarConnection.getJarFile(); + _file = new File(_jarFile.getName()); + } } /** @@ -253,36 +263,39 @@ public class JarFileResource extends JarResource } @Override - public synchronized String[] list() + public String[] list() { - if (isDirectory() && _list == null) + try (AutoLock ignored = _lock.lock()) { - List list = null; - try + if (isDirectory() && _list == null) { - list = listEntries(); - } - catch (Exception e) - { - //Sun's JarURLConnection impl for jar: protocol will close a JarFile in its connect() method if - //useCaches == false (eg someone called URLConnection with defaultUseCaches==true). - //As their sun.net.www.protocol.jar package caches JarFiles and/or connections, we can wind up in - //the situation where the JarFile we have remembered in our _jarFile member has actually been closed - //by other code. - //So, do one retry to drop a connection and get a fresh JarFile - LOG.warn("Retrying list:" + e); - LOG.debug("JarFile list failure", e); - close(); - list = listEntries(); - } + List list = null; + try + { + list = listEntries(); + } + catch (Exception e) + { + //Sun's JarURLConnection impl for jar: protocol will close a JarFile in its connect() method if + //useCaches == false (eg someone called URLConnection with defaultUseCaches==true). + //As their sun.net.www.protocol.jar package caches JarFiles and/or connections, we can wind up in + //the situation where the JarFile we have remembered in our _jarFile member has actually been closed + //by other code. + //So, do one retry to drop a connection and get a fresh JarFile + LOG.warn("Retrying list:" + e); + LOG.debug("JarFile list failure", e); + close(); + list = listEntries(); + } - if (list != null) - { - _list = new String[list.size()]; - list.toArray(_list); + if (list != null) + { + _list = new String[list.size()]; + list.toArray(_list); + } } + return _list; } - return _list; } private List listEntries() diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java index ec7e925aec3..276df113b37 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/JarResource.java @@ -34,12 +34,14 @@ import java.util.jar.Manifest; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class JarResource extends URLResource { private static final Logger LOG = LoggerFactory.getLogger(JarResource.class); + protected JarURLConnection _jarConnection; protected JarResource(URL url) @@ -53,28 +55,34 @@ public class JarResource extends URLResource } @Override - public synchronized void close() + public void close() { - _jarConnection = null; - super.close(); + try (AutoLock ignored = _lock.lock()) + { + _jarConnection = null; + super.close(); + } } @Override - protected synchronized boolean checkConnection() + protected boolean checkConnection() { - super.checkConnection(); - try + try (AutoLock ignored = _lock.lock()) { - if (_jarConnection != _connection) - newConnection(); - } - catch (IOException e) - { - LOG.trace("IGNORED", e); - _jarConnection = null; - } + super.checkConnection(); + try + { + if (_jarConnection != _connection) + newConnection(); + } + catch (IOException e) + { + LOG.trace("IGNORED", e); + _jarConnection = null; + } - return _jarConnection != null; + return _jarConnection != null; + } } /** diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java index 7561dd88b05..203ab064b8a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/resource/URLResource.java @@ -29,6 +29,7 @@ import java.net.URLConnection; import java.nio.channels.ReadableByteChannel; import org.eclipse.jetty.util.URIUtil; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -38,9 +39,10 @@ import org.slf4j.LoggerFactory; public class URLResource extends Resource { private static final Logger LOG = LoggerFactory.getLogger(URLResource.class); + + protected final AutoLock _lock = new AutoLock(); protected final URL _url; protected final String _urlString; - protected URLConnection _connection; protected InputStream _in = null; transient boolean _useCaches = Resource.__defaultUseCaches; @@ -58,44 +60,50 @@ public class URLResource extends Resource _useCaches = useCaches; } - protected synchronized boolean checkConnection() + protected boolean checkConnection() { - if (_connection == null) + try (AutoLock ignored = _lock.lock()) { - try + if (_connection == null) { - _connection = _url.openConnection(); - _connection.setUseCaches(_useCaches); - } - catch (IOException e) - { - LOG.trace("IGNORED", e); + try + { + _connection = _url.openConnection(); + _connection.setUseCaches(_useCaches); + } + catch (IOException e) + { + LOG.trace("IGNORED", e); + } } + return _connection != null; } - return _connection != null; } /** * Release any resources held by the resource. */ @Override - public synchronized void close() + public void close() { - if (_in != null) + try (AutoLock ignored = _lock.lock()) { - try + if (_in != null) { - _in.close(); + try + { + _in.close(); + } + catch (IOException e) + { + LOG.trace("IGNORED", e); + } + _in = null; } - catch (IOException e) - { - LOG.trace("IGNORED", e); - } - _in = null; - } - if (_connection != null) - _connection = null; + if (_connection != null) + _connection = null; + } } /** @@ -106,7 +114,7 @@ public class URLResource extends Resource { try { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (checkConnection() && _in == null) _in = _connection.getInputStream(); @@ -193,8 +201,7 @@ public class URLResource extends Resource * url connection will be nulled out to prevent re-use. */ @Override - public synchronized InputStream getInputStream() - throws java.io.IOException + public InputStream getInputStream() throws IOException { return getInputStream(true); //backwards compatibility } @@ -210,29 +217,31 @@ public class URLResource extends Resource * @return the inputstream for this resource * @throws IOException if unable to open the input stream */ - protected synchronized InputStream getInputStream(boolean resetConnection) - throws IOException + protected InputStream getInputStream(boolean resetConnection) throws IOException { - if (!checkConnection()) - throw new IOException("Invalid resource"); + try (AutoLock ignored = _lock.lock()) + { + if (!checkConnection()) + throw new IOException("Invalid resource"); - try - { - if (_in != null) + try { - InputStream in = _in; - _in = null; - return in; + if (_in != null) + { + InputStream in = _in; + _in = null; + return in; + } + return _connection.getInputStream(); } - return _connection.getInputStream(); - } - finally - { - if (resetConnection) + finally { - _connection = null; - if (LOG.isDebugEnabled()) - LOG.debug("Connection nulled"); + if (resetConnection) + { + _connection = null; + if (LOG.isDebugEnabled()) + LOG.debug("Connection nulled"); + } } } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java b/jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java index 46aad34040e..03bac4d9d13 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/security/Credential.java @@ -185,8 +185,6 @@ public abstract class Credential implements Serializable { private static final long serialVersionUID = 5533846540822684240L; private static final String __TYPE = "MD5:"; - private static final Object __md5Lock = new Object(); - private static MessageDigest __md; private final byte[] _digest; @@ -210,15 +208,8 @@ public abstract class Credential implements Serializable credentials = new String((char[])credentials); if (credentials instanceof Password || credentials instanceof String) { - byte[] digest; - synchronized (__md5Lock) - { - if (__md == null) - __md = MessageDigest.getInstance("MD5"); - __md.reset(); - __md.update(credentials.toString().getBytes(StandardCharsets.ISO_8859_1)); - digest = __md.digest(); - } + MessageDigest md5 = MessageDigest.getInstance("MD5"); + byte[] digest = md5.digest(credentials.toString().getBytes(StandardCharsets.ISO_8859_1)); return byteEquals(_digest, digest); } else if (credentials instanceof MD5) @@ -256,27 +247,8 @@ public abstract class Credential implements Serializable { try { - byte[] digest; - synchronized (__md5Lock) - { - if (__md == null) - { - try - { - __md = MessageDigest.getInstance("MD5"); - } - catch (Exception e) - { - LOG.warn("Unable to access MD5 message digest", e); - return null; - } - } - - __md.reset(); - __md.update(password.getBytes(StandardCharsets.ISO_8859_1)); - digest = __md.digest(); - } - + MessageDigest md5 = MessageDigest.getInstance("MD5"); + byte[] digest = md5.digest(password.getBytes(StandardCharsets.ISO_8859_1)); return __TYPE + TypeUtil.toString(digest, 16); } catch (Exception e) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java index 8f1efd1af52..45e54a865a5 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ssl/SslContextFactory.java @@ -90,6 +90,7 @@ import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.security.CertificateUtils; import org.eclipse.jetty.util.security.CertificateValidator; import org.eclipse.jetty.util.security.Password; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -137,6 +138,7 @@ public abstract class SslContextFactory extends AbstractLifeCycle implements Dum "^.*_anon_.*$" }; + private final AutoLock _lock = new AutoLock(); private final Set _excludeProtocols = new LinkedHashSet<>(); private final Set _includeProtocols = new LinkedHashSet<>(); private final Set _excludeCipherSuites = new LinkedHashSet<>(); @@ -212,7 +214,7 @@ public abstract class SslContextFactory extends AbstractLifeCycle implements Dum protected void doStart() throws Exception { super.doStart(); - synchronized (this) + try (AutoLock ignored = _lock.lock()) { load(); } @@ -436,7 +438,7 @@ public abstract class SslContextFactory extends AbstractLifeCycle implements Dum @Override protected void doStop() throws Exception { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { unload(); } @@ -1039,11 +1041,10 @@ public abstract class SslContextFactory extends AbstractLifeCycle implements Dum if (!isStarted()) return _setContext; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_factory == null) throw new IllegalStateException("SslContextFactory reload failed"); - return _factory._context; } } @@ -1421,11 +1422,10 @@ public abstract class SslContextFactory extends AbstractLifeCycle implements Dum if (!isStarted()) return _setKeyStore; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_factory == null) throw new IllegalStateException("SslContextFactory reload failed"); - return _factory._keyStore; } } @@ -1445,11 +1445,10 @@ public abstract class SslContextFactory extends AbstractLifeCycle implements Dum if (!isStarted()) return _setTrustStore; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_factory == null) throw new IllegalStateException("SslContextFactory reload failed"); - return _factory._trustStore; } } @@ -1883,7 +1882,7 @@ public abstract class SslContextFactory extends AbstractLifeCycle implements Dum public void reload(Consumer consumer) throws Exception { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { consumer.accept(this); unload(); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/statistic/RateStatistic.java b/jetty-util/src/main/java/org/eclipse/jetty/util/statistic/RateStatistic.java index 9743467c82b..784be205528 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/statistic/RateStatistic.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/statistic/RateStatistic.java @@ -23,6 +23,8 @@ import java.util.Deque; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import org.eclipse.jetty.util.thread.AutoLock; + /** *

Statistics on a time sequence rate.

*

Calculates the rate at which the {@link #record()} method is called @@ -34,6 +36,7 @@ import java.util.stream.Collectors; */ public class RateStatistic { + private final AutoLock _lock = new AutoLock(); private final Deque _samples = new ArrayDeque<>(); private final long _nanoPeriod; private final TimeUnit _units; @@ -61,7 +64,7 @@ public class RateStatistic */ public void reset() { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _samples.clear(); _max = 0; @@ -88,7 +91,7 @@ public class RateStatistic protected void age(long period, TimeUnit units) { long increment = TimeUnit.NANOSECONDS.convert(period, units); - synchronized (this) + try (AutoLock ignored = _lock.lock()) { int size = _samples.size(); for (int i = 0; i < size; i++) @@ -107,7 +110,7 @@ public class RateStatistic public int record() { long now = System.nanoTime(); - synchronized (this) + try (AutoLock ignored = _lock.lock()) { _count++; _samples.add(now); @@ -124,7 +127,7 @@ public class RateStatistic */ public int getRate() { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { update(); return _samples.size(); @@ -136,10 +139,7 @@ public class RateStatistic */ public long getMax() { - synchronized (this) - { - return _max; - } + return _lock.runLocked(() -> _max); } /** @@ -148,7 +148,7 @@ public class RateStatistic */ public long getOldest(TimeUnit units) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { Long head = _samples.peekFirst(); if (head == null) @@ -162,10 +162,7 @@ public class RateStatistic */ public long getCount() { - synchronized (this) - { - return _count; - } + return _lock.runLocked(() -> _count); } public String dump() @@ -176,7 +173,7 @@ public class RateStatistic public String dump(TimeUnit units) { long now = System.nanoTime(); - synchronized (this) + try (AutoLock ignored = _lock.lock()) { String samples = _samples.stream() .mapToLong(t -> units.convert(now - t, TimeUnit.NANOSECONDS)) @@ -194,7 +191,7 @@ public class RateStatistic private String toString(long nanoTime) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { update(nanoTime); return String.format("%s@%x{count=%d,max=%d,rate=%d per %d %s}", diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/AutoLock.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/AutoLock.java index 34f9aebdaf9..bac7edbf626 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/AutoLock.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/AutoLock.java @@ -18,11 +18,15 @@ package org.eclipse.jetty.util.thread; +import java.io.Serializable; +import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Supplier; /** - * Reentrant lock that can be used in a try-with-resources statement. + *

Reentrant lock that can be used in a try-with-resources statement.

+ *

Typical usage:

* 
  * try (AutoLock lock = this.lock.lock())
  * {
@@ -30,7 +34,7 @@ import java.util.concurrent.locks.ReentrantLock;
  * }
  *
*/ -public class AutoLock implements AutoCloseable +public class AutoLock implements AutoCloseable, Serializable { private final ReentrantLock _lock = new ReentrantLock(); @@ -45,6 +49,57 @@ public class AutoLock implements AutoCloseable return this; } + /** + *

Runs the given code with the lock held.

+ *

This is equivalent to:

+ * 
+     * try (AutoLock ignored = lock())
+     * {
+     *     code.run();
+     * }
+     *
+ * + * @param code the code to run with the lock held. + */ + public void runLocked(Runnable code) + { + try (AutoLock ignored = lock()) + { + code.run(); + } + } + + /** + *

Returns the result of running the given code with the lock held.

+ *

This is equivalent to:

+ * 
+     * try (AutoLock ignored = lock())
+     * {
+     *     return code.get();
+     * }
+     *
+ * + * @param code the code to run with the lock held. + * @param the result type + * @return the result of the code run + */ + public T runLocked(Supplier code) + { + try (AutoLock ignored = lock()) + { + return code.get(); + } + } + + /** + * @see ReentrantLock#isHeldByCurrentThread() + * @return whether this lock is held by the current thread + */ + public boolean isHeldByCurrentThread() + { + return _lock.isHeldByCurrentThread(); + } + /** * @return a {@link Condition} associated with this lock */ @@ -64,4 +119,69 @@ public class AutoLock implements AutoCloseable { _lock.unlock(); } + + /** + *

A reentrant lock with a condition that can be used in a try-with-resources statement.

+ *

Typical usage:

+ * 
+     * // Waiting
+     * try (AutoLock lock = _lock.lock())
+     * {
+     *     lock.await();
+     * }
+     *
+     * // Signaling
+     * try (AutoLock lock = _lock.lock())
+     * {
+     *     lock.signalAll();
+     * }
+     *
+ */ + public static class WithCondition extends AutoLock + { + private final Condition _condition = newCondition(); + + @Override + public AutoLock.WithCondition lock() + { + return (WithCondition)super.lock(); + } + + /** + * @see Condition#signal() + */ + public void signal() + { + _condition.signal(); + } + + /** + * @see Condition#signalAll() + */ + public void signalAll() + { + _condition.signalAll(); + } + + /** + * @see Condition#await() + * @throws InterruptedException if the current thread is interrupted + */ + public void await() throws InterruptedException + { + _condition.await(); + } + + /** + * @see Condition#await(long, TimeUnit) + * @param time the time to wait + * @param unit the time unit + * @return false if the waiting time elapsed + * @throws InterruptedException if the current thread is interrupted + */ + public boolean await(long time, TimeUnit unit) throws InterruptedException + { + return _condition.await(time, unit); + } + } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index c4fab5abbfa..6accd65fe3b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -64,7 +64,7 @@ public class QueuedThreadPool extends ContainerLifeCycle implements ThreadFactor private final AtomicBiInteger _counts = new AtomicBiInteger(Integer.MIN_VALUE, 0); private final AtomicLong _lastShrink = new AtomicLong(); private final Set _threads = ConcurrentHashMap.newKeySet(); - private final Object _joinLock = new Object(); + private final AutoLock.WithCondition _joinLock = new AutoLock.WithCondition(); private final BlockingQueue _jobs; private final ThreadGroup _threadGroup; private final ThreadFactory _threadFactory; @@ -277,9 +277,9 @@ public class QueuedThreadPool extends ContainerLifeCycle implements ThreadFactor if (_budget != null) _budget.reset(); - synchronized (_joinLock) + try (AutoLock.WithCondition lock = _joinLock.lock()) { - _joinLock.notifyAll(); + lock.signalAll(); } } @@ -570,11 +570,11 @@ public class QueuedThreadPool extends ContainerLifeCycle implements ThreadFactor @Override public void join() throws InterruptedException { - synchronized (_joinLock) + try (AutoLock.WithCondition lock = _joinLock.lock()) { while (isRunning()) { - _joinLock.wait(); + lock.await(); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ShutdownThread.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ShutdownThread.java index c38fa02a63b..be74c189ba3 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ShutdownThread.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ShutdownThread.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.util.thread; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; @@ -36,10 +37,11 @@ import org.slf4j.LoggerFactory; public class ShutdownThread extends Thread { private static final Logger LOG = LoggerFactory.getLogger(ShutdownThread.class); - private static final ShutdownThread _thread = new ShutdownThread(); + private static final ShutdownThread INSTANCE = new ShutdownThread(); + private final AutoLock _lock = new AutoLock(); + private final List _lifeCycles = new CopyOnWriteArrayList<>(); private boolean _hooked; - private final List _lifeCycles = new CopyOnWriteArrayList(); /** * Default constructor for the singleton @@ -51,9 +53,9 @@ public class ShutdownThread extends Thread super("JettyShutdownThread"); } - private synchronized void hook() + private void hook() { - try + try (AutoLock ignored = _lock.lock()) { if (!_hooked) Runtime.getRuntime().addShutdownHook(this); @@ -66,9 +68,9 @@ public class ShutdownThread extends Thread } } - private synchronized void unhook() + private void unhook() { - try + try (AutoLock ignored = _lock.lock()) { _hooked = false; Runtime.getRuntime().removeShutdownHook(this); @@ -81,45 +83,53 @@ public class ShutdownThread extends Thread } /** - * Returns the instance of the singleton - * - * @return the singleton instance of the {@link ShutdownThread} + * @return the singleton instance of the ShutdownThread */ public static ShutdownThread getInstance() { - return _thread; + return INSTANCE; } - public static synchronized void register(LifeCycle... lifeCycles) + public static void register(LifeCycle... lifeCycles) { - _thread._lifeCycles.addAll(Arrays.asList(lifeCycles)); - if (_thread._lifeCycles.size() > 0) - _thread.hook(); + try (AutoLock ignored = INSTANCE._lock.lock()) + { + INSTANCE._lifeCycles.addAll(Arrays.asList(lifeCycles)); + if (INSTANCE._lifeCycles.size() > 0) + INSTANCE.hook(); + } } - public static synchronized void register(int index, LifeCycle... lifeCycles) + public static void register(int index, LifeCycle... lifeCycles) { - _thread._lifeCycles.addAll(index, Arrays.asList(lifeCycles)); - if (_thread._lifeCycles.size() > 0) - _thread.hook(); + try (AutoLock ignored = INSTANCE._lock.lock()) + { + INSTANCE._lifeCycles.addAll(index, Arrays.asList(lifeCycles)); + if (INSTANCE._lifeCycles.size() > 0) + INSTANCE.hook(); + } } - public static synchronized void deregister(LifeCycle lifeCycle) + public static void deregister(LifeCycle lifeCycle) { - _thread._lifeCycles.remove(lifeCycle); - if (_thread._lifeCycles.size() == 0) - _thread.unhook(); + try (AutoLock ignored = INSTANCE._lock.lock()) + { + INSTANCE._lifeCycles.remove(lifeCycle); + if (INSTANCE._lifeCycles.size() == 0) + INSTANCE.unhook(); + } } - public static synchronized boolean isRegistered(LifeCycle lifeCycle) + public static boolean isRegistered(LifeCycle lifeCycle) { - return _thread._lifeCycles.contains(lifeCycle); + return INSTANCE._lock.runLocked(() -> INSTANCE._lifeCycles.contains(lifeCycle)); } @Override public void run() { - for (LifeCycle lifeCycle : _thread._lifeCycles) + List lifeCycles = INSTANCE._lock.runLocked(() -> new ArrayList<>(INSTANCE._lifeCycles)); + for (LifeCycle lifeCycle : lifeCycles) { try { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/EatWhatYouKill.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/EatWhatYouKill.java index b295092fc06..5b2cb557246 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/EatWhatYouKill.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/EatWhatYouKill.java @@ -29,6 +29,7 @@ import org.eclipse.jetty.util.annotation.ManagedAttribute; import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.annotation.ManagedOperation; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ExecutionStrategy; import org.eclipse.jetty.util.thread.Invocable; import org.eclipse.jetty.util.thread.TryExecutor; @@ -72,6 +73,7 @@ public class EatWhatYouKill extends ContainerLifeCycle implements ExecutionStrat EXECUTE_PRODUCE_CONSUME // Eat What You Kill! } + private final AutoLock _lock = new AutoLock(); private final LongAdder _pcMode = new LongAdder(); private final LongAdder _picMode = new LongAdder(); private final LongAdder _pecMode = new LongAdder(); @@ -97,7 +99,7 @@ public class EatWhatYouKill extends ContainerLifeCycle implements ExecutionStrat public void dispatch() { boolean execute = false; - synchronized (this) + try (AutoLock ignored = _lock.lock()) { switch (_state) { @@ -140,7 +142,7 @@ public class EatWhatYouKill extends ContainerLifeCycle implements ExecutionStrat if (LOG.isDebugEnabled()) LOG.debug("{} tryProduce {}", this, wasPending); - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (wasPending) _pending = false; @@ -185,7 +187,7 @@ public class EatWhatYouKill extends ContainerLifeCycle implements ExecutionStrat if (task == null) { - synchronized (this) + try (AutoLock ignored = _lock.lock()) { // Could another task just have been queued with a produce call? switch (_state) @@ -237,7 +239,7 @@ public class EatWhatYouKill extends ContainerLifeCycle implements ExecutionStrat case BLOCKING: // The task is blocking, so PC is not an option. Thus we choose // between EPC and PEC based on the availability of a reserved thread. - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_pending) { @@ -260,7 +262,7 @@ public class EatWhatYouKill extends ContainerLifeCycle implements ExecutionStrat case EITHER: // The task may be non blocking, so PC is an option. Thus we choose // between EPC and PC based on the availability of a reserved thread. - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_pending) { @@ -313,7 +315,7 @@ public class EatWhatYouKill extends ContainerLifeCycle implements ExecutionStrat runTask(task); // Try to produce again? - synchronized (this) + try (AutoLock ignored = _lock.lock()) { if (_state == State.IDLE) { @@ -420,10 +422,7 @@ public class EatWhatYouKill extends ContainerLifeCycle implements ExecutionStrat @ManagedAttribute(value = "whether this execution strategy is idle", readonly = true) public boolean isIdle() { - synchronized (this) - { - return _state == State.IDLE; - } + return _lock.runLocked(() -> _state == State.IDLE); } @ManagedOperation(value = "resets the task counts", impact = "ACTION") @@ -438,10 +437,7 @@ public class EatWhatYouKill extends ContainerLifeCycle implements ExecutionStrat @Override public String toString() { - synchronized (this) - { - return toStringLocked(); - } + return _lock.runLocked(this::toStringLocked); } public String toStringLocked() diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Configurations.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Configurations.java index c43eac366bd..aaf98302a39 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Configurations.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/Configurations.java @@ -41,6 +41,7 @@ import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.annotation.Name; import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.DumpableCollection; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -66,15 +67,18 @@ import org.slf4j.LoggerFactory; public class Configurations extends AbstractList implements Dumpable { private static final Logger LOG = LoggerFactory.getLogger(Configurations.class); - + private static final AutoLock __lock = new AutoLock(); private static final List __known = new ArrayList<>(); private static final List __unavailable = new ArrayList<>(); private static final Set __knownByClassName = new HashSet<>(); - public static synchronized List getKnown() + public static List getKnown() { - if (__known.isEmpty()) + try (AutoLock ignored = __lock.lock()) { + if (!__known.isEmpty()) + return __known; + TypeUtil.serviceProviderStream(ServiceLoader.load(Configuration.class)).forEach(provider -> { try @@ -95,7 +99,6 @@ public class Configurations extends AbstractList implements Dumpa LOG.warn("Unable to get known Configuration", e); } }); - sort(__known); if (LOG.isDebugEnabled()) { @@ -105,53 +108,62 @@ public class Configurations extends AbstractList implements Dumpa } } - LOG.debug("Known Configurations {}", __knownByClassName); + if (LOG.isDebugEnabled()) + LOG.debug("Known Configurations {}", __knownByClassName); + return __known; } - return __known; } - public static synchronized void setKnown(String... classes) + public static void setKnown(String... classes) { - if (!__known.isEmpty()) - throw new IllegalStateException("Known configuration classes already set"); - - for (String c : classes) + try (AutoLock ignored = __lock.lock()) { - try + if (!__known.isEmpty()) + throw new IllegalStateException("Known configuration classes already set"); + + for (String c : classes) { - Class clazz = Loader.loadClass(c); - Configuration configuration = clazz.getConstructor().newInstance(); - if (!configuration.isAvailable()) + try { - if (LOG.isDebugEnabled()) - LOG.warn("Configuration unavailable: " + configuration); - __unavailable.add(configuration); - continue; + Class clazz = Loader.loadClass(c); + Configuration configuration = clazz.getConstructor().newInstance(); + if (!configuration.isAvailable()) + { + if (LOG.isDebugEnabled()) + LOG.warn("Configuration unavailable: " + configuration); + __unavailable.add(configuration); + continue; + } + __known.add(clazz.getConstructor().newInstance()); + __knownByClassName.add(c); + } + catch (Exception e) + { + LOG.warn("Problem loading known class", e); } - __known.add(clazz.getConstructor().newInstance()); - __knownByClassName.add(c); } - catch (Exception e) + sort(__known); + if (LOG.isDebugEnabled()) { - LOG.warn("Problem loading known class", e); + for (Configuration c : __known) + { + LOG.debug("known {}", c); + } } - } - sort(__known); - if (LOG.isDebugEnabled()) - { - for (Configuration c : __known) - { - LOG.debug("known {}", c); - } - } - LOG.debug("Known Configurations {}", __knownByClassName); + if (LOG.isDebugEnabled()) + LOG.debug("Known Configurations {}", __knownByClassName); + } } - static synchronized void cleanKnown() + // Only used by tests. + static void clearKnown() { - __known.clear(); - __unavailable.clear(); + try (AutoLock ignored = __lock.lock()) + { + __known.clear(); + __unavailable.clear(); + } } /** diff --git a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java index 5afc94ae9b9..b778e665718 100644 --- a/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java +++ b/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/MetaData.java @@ -29,6 +29,7 @@ import javax.servlet.ServletContext; import org.eclipse.jetty.util.resource.EmptyResource; import org.eclipse.jetty.util.resource.Resource; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,6 +46,7 @@ public class MetaData public static final String ORDERED_LIBS = "javax.servlet.context.orderedLibs"; public static final Resource NON_FRAG_RESOURCE = EmptyResource.INSTANCE; + private final AutoLock _lock = new AutoLock(); protected Map _origins = new HashMap<>(); protected WebDescriptor _webDefaultsRoot; protected WebDescriptor _webXmlRoot; @@ -360,41 +362,38 @@ public class MetaData * * @param annotation the discovered annotation */ - public synchronized void addDiscoveredAnnotation(DiscoveredAnnotation annotation) + public void addDiscoveredAnnotation(DiscoveredAnnotation annotation) { if (annotation == null) return; - //if no resource associated with an annotation map it to empty resource - these - //annotations will always be processed first - Resource enclosingResource = EmptyResource.INSTANCE; - Resource resource = annotation.getResource(); - if (resource != null) + try (AutoLock ignored = _lock.lock()) { - //check if any of the web-inf classes dirs is a parent - enclosingResource = getEnclosingResource(_webInfClasses, resource); + //if no resource associated with an annotation map it to empty resource - these + //annotations will always be processed first + Resource enclosingResource = EmptyResource.INSTANCE; + Resource resource = annotation.getResource(); + if (resource != null) + { + //check if any of the web-inf classes dirs is a parent + enclosingResource = getEnclosingResource(_webInfClasses, resource); - //check if any of the web-inf jars is a parent - if (enclosingResource == null) - enclosingResource = getEnclosingResource(_webInfJars, resource); + //check if any of the web-inf jars is a parent + if (enclosingResource == null) + enclosingResource = getEnclosingResource(_webInfJars, resource); - //check if any of the container resources is a parent - if (enclosingResource == null) - enclosingResource = getEnclosingResource(_orderedContainerResources, resource); + //check if any of the container resources is a parent + if (enclosingResource == null) + enclosingResource = getEnclosingResource(_orderedContainerResources, resource); - //Couldn't find a parent resource in any of the known resources, map it to the empty resource - if (enclosingResource == null) - enclosingResource = EmptyResource.INSTANCE; + //Couldn't find a parent resource in any of the known resources, map it to the empty resource + if (enclosingResource == null) + enclosingResource = EmptyResource.INSTANCE; + } + + List list = _annotations.computeIfAbsent(enclosingResource, k -> new ArrayList<>()); + list.add(annotation); } - - List list = _annotations.get(enclosingResource); - if (list == null) - { - list = new ArrayList<>(); - _annotations.put(enclosingResource, list); - } - - list.add(annotation); } /** diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/ConfigurationsTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/ConfigurationsTest.java index 02c8632211e..c0c4622969b 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/ConfigurationsTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/ConfigurationsTest.java @@ -34,13 +34,13 @@ public class ConfigurationsTest @AfterEach public void tearDown() { - Configurations.cleanKnown(); + Configurations.clearKnown(); } @BeforeEach public void setup() { - Configurations.cleanKnown(); + Configurations.clearKnown(); } @Test diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java index 17c9fc6336c..481923fa079 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java @@ -76,7 +76,7 @@ public class WebAppContextTest @AfterEach public void tearDown() { - Configurations.cleanKnown(); + Configurations.clearKnown(); } @Test @@ -157,7 +157,7 @@ public class WebAppContextTest @Test public void testConfigurationClassesFromDefault() { - Configurations.cleanKnown(); + Configurations.clearKnown(); String[] knownAndEnabled = Configurations.getKnown().stream() .filter(c -> c.isEnabledByDefault()) .map(c -> c.getClass().getName()) @@ -182,7 +182,7 @@ public class WebAppContextTest @Test public void testConfigurationOrder() { - Configurations.cleanKnown(); + Configurations.clearKnown(); WebAppContext wac = new WebAppContext(); wac.setServer(new Server()); assertThat(wac.getConfigurations().stream().map(c -> c.getClass().getName()).collect(Collectors.toList()), @@ -199,7 +199,7 @@ public class WebAppContextTest @Test public void testConfigurationInstances() { - Configurations.cleanKnown(); + Configurations.clearKnown(); Configuration[] configs = {new WebInfConfiguration()}; WebAppContext wac = new WebAppContext(); wac.setConfigurations(configs); diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameFlusher.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameFlusher.java index e4483f6e719..f763c961cd2 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameFlusher.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameFlusher.java @@ -35,6 +35,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IteratingCallback; import org.eclipse.jetty.util.TypeUtil; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Scheduler; import org.eclipse.jetty.websocket.core.CloseStatus; import org.eclipse.jetty.websocket.core.Frame; @@ -50,6 +51,7 @@ public class FrameFlusher extends IteratingCallback private static final Logger LOG = LoggerFactory.getLogger(FrameFlusher.class); private static final Throwable CLOSED_CHANNEL = new ClosedChannelException(); + private final AutoLock lock = new AutoLock(); private final LongAdder messagesOut = new LongAdder(); private final LongAdder bytesOut = new LongAdder(); private final ByteBufferPool bufferPool; @@ -114,7 +116,7 @@ public class FrameFlusher extends IteratingCallback List failedEntries = null; CloseStatus closeStatus = null; - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (canEnqueue) { @@ -186,11 +188,7 @@ public class FrameFlusher extends IteratingCallback public void onClose(Throwable cause) { - synchronized (this) - { - closedCause = cause == null ? CLOSED_CHANNEL : cause; - } - + lock.runLocked(() -> closedCause = cause == null ? CLOSED_CHANNEL : cause); iterate(); } @@ -202,7 +200,7 @@ public class FrameFlusher extends IteratingCallback boolean flush = false; Callback releasingCallback = this; - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (closedCause != null) throw closedCause; @@ -349,16 +347,13 @@ public class FrameFlusher extends IteratingCallback private int getQueueSize() { - synchronized (this) - { - return queue.size(); - } + return lock.runLocked(queue::size); } public void timeoutExpired() { boolean failed = false; - synchronized (FrameFlusher.this) + try (AutoLock ignored = lock.lock()) { if (closedCause != null) return; @@ -409,7 +404,7 @@ public class FrameFlusher extends IteratingCallback { BufferUtil.clear(batchBuffer); releaseAggregate(); - synchronized (this) + try (AutoLock ignored = lock.lock()) { failedEntries.addAll(queue); queue.clear(); diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameSequence.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameSequence.java index 5bc9d128270..c2d1ba278e8 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameSequence.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameSequence.java @@ -18,17 +18,19 @@ package org.eclipse.jetty.websocket.core.internal; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.websocket.core.OpCode; import org.eclipse.jetty.websocket.core.exception.ProtocolException; public class FrameSequence { + private final AutoLock lock = new AutoLock(); // TODO should we be able to get a non fin frame then get a close frame without error private byte state = OpCode.UNDEFINED; public void check(byte opcode, boolean fin) throws ProtocolException { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (state == OpCode.CLOSE) throw new ProtocolException(OpCode.name(opcode) + " after CLOSE"); diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/TransformingFlusher.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/TransformingFlusher.java index 28d30165aa3..b73166ab2ce 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/TransformingFlusher.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/TransformingFlusher.java @@ -23,6 +23,7 @@ import java.util.Queue; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IteratingCallback; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.websocket.core.Frame; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -38,6 +39,7 @@ public abstract class TransformingFlusher { private final Logger log = LoggerFactory.getLogger(this.getClass()); + private final AutoLock lock = new AutoLock(); private final Queue entries = new ArrayDeque<>(); private final IteratingCallback flusher = new Flusher(); private boolean finished = true; @@ -68,12 +70,10 @@ public abstract class TransformingFlusher log.debug("Queuing {}", entry); boolean enqueued = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (failure == null) - { enqueued = entries.add(entry); - } } if (enqueued) @@ -84,7 +84,7 @@ public abstract class TransformingFlusher private void onFailure(Throwable t) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (failure == null) failure = t; @@ -97,10 +97,7 @@ public abstract class TransformingFlusher private FrameEntry pollEntry() { - synchronized (this) - { - return entries.poll(); - } + return lock.runLocked(entries::poll); } private class Flusher extends IteratingCallback implements Callback @@ -137,7 +134,7 @@ public abstract class TransformingFlusher protected void onCompleteFailure(Throwable t) { if (log.isDebugEnabled()) - log.debug("failed {}", t); + log.debug("failed to flush", t); notifyCallbackFailure(current.callback, t); current = null; diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketConnection.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketConnection.java index 74497c7a235..d13f6626b31 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketConnection.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketConnection.java @@ -35,6 +35,7 @@ import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.component.Dumpable; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Scheduler; import org.eclipse.jetty.websocket.core.Behavior; import org.eclipse.jetty.websocket.core.Frame; @@ -54,19 +55,17 @@ public class WebSocketConnection extends AbstractConnection implements Connectio */ private static final int MIN_BUFFER_SIZE = Generator.MAX_HEADER_LENGTH; + private final AutoLock lock = new AutoLock(); private final ByteBufferPool bufferPool; private final Generator generator; private final Parser parser; private final WebSocketCoreSession coreSession; - private final Flusher flusher; private final Random random; - private long demand; private boolean fillingAndParsing; private final LongAdder messagesIn = new LongAdder(); private final LongAdder bytesIn = new LongAdder(); - // Read / Parse variables private RetainableByteBuffer networkBuffer; private boolean useInputDirectByteBuffers; @@ -265,7 +264,7 @@ public class WebSocketConnection extends AbstractConnection implements Connectio private void acquireNetworkBuffer() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (networkBuffer == null) networkBuffer = newNetworkBuffer(getInputBufferSize()); @@ -274,7 +273,7 @@ public class WebSocketConnection extends AbstractConnection implements Connectio private void reacquireNetworkBuffer() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (networkBuffer == null) throw new IllegalStateException(); @@ -294,7 +293,7 @@ public class WebSocketConnection extends AbstractConnection implements Connectio private void releaseNetworkBuffer() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (networkBuffer == null) throw new IllegalStateException(); @@ -329,7 +328,7 @@ public class WebSocketConnection extends AbstractConnection implements Connectio throw new IllegalArgumentException("Demand must be positive"); boolean fillAndParse = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug("demand {} d={} fp={} {} {}", n, demand, fillingAndParsing, networkBuffer, this); @@ -362,7 +361,7 @@ public class WebSocketConnection extends AbstractConnection implements Connectio public boolean moreDemand() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug("moreDemand? d={} fp={} {} {}", demand, fillingAndParsing, networkBuffer, this); @@ -382,7 +381,7 @@ public class WebSocketConnection extends AbstractConnection implements Connectio public boolean meetDemand() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug("meetDemand d={} fp={} {} {}", demand, fillingAndParsing, networkBuffer, this); @@ -401,11 +400,10 @@ public class WebSocketConnection extends AbstractConnection implements Connectio public void cancelDemand() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (LOG.isDebugEnabled()) LOG.debug("cancelDemand d={} fp={} {} {}", demand, fillingAndParsing, networkBuffer, this); - demand = -1; } } @@ -489,10 +487,7 @@ public class WebSocketConnection extends AbstractConnection implements Connectio { if (LOG.isDebugEnabled()) LOG.debug("Set initial buffer - {}", BufferUtil.toDetailString(initialBuffer)); - synchronized (this) - { - networkBuffer = newNetworkBuffer(initialBuffer.remaining()); - } + lock.runLocked(() -> networkBuffer = newNetworkBuffer(initialBuffer.remaining())); ByteBuffer buffer = networkBuffer.getBuffer(); BufferUtil.clearToFill(buffer); BufferUtil.put(initialBuffer, buffer); diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java index 6054ab8068f..a2926af60fe 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.websocket.core.internal; import java.nio.channels.ClosedChannelException; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.websocket.core.CloseStatus; import org.eclipse.jetty.websocket.core.Frame; import org.eclipse.jetty.websocket.core.OpCode; @@ -41,6 +42,7 @@ public class WebSocketSessionState CLOSED } + private final AutoLock lock = new AutoLock(); private State _sessionState = State.CONNECTING; private byte _incomingContinuation = OpCode.UNDEFINED; private byte _outgoingContinuation = OpCode.UNDEFINED; @@ -48,7 +50,7 @@ public class WebSocketSessionState public void onConnected() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (_sessionState != State.CONNECTING) throw new IllegalStateException(_sessionState.toString()); @@ -59,7 +61,7 @@ public class WebSocketSessionState public void onOpen() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { switch (_sessionState) { @@ -78,12 +80,9 @@ public class WebSocketSessionState } } - public State getState() + private State getState() { - synchronized (this) - { - return _sessionState; - } + return lock.runLocked(() -> _sessionState); } public boolean isClosed() @@ -105,15 +104,12 @@ public class WebSocketSessionState public CloseStatus getCloseStatus() { - synchronized (this) - { - return _closeStatus; - } + return lock.runLocked(() -> _closeStatus); } public boolean onClosed(CloseStatus closeStatus) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (_sessionState == State.CLOSED) return false; @@ -142,7 +138,7 @@ public class WebSocketSessionState */ public void onError(Throwable t) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (_sessionState != State.CLOSED || _closeStatus == null) throw new IllegalArgumentException(); @@ -159,7 +155,7 @@ public class WebSocketSessionState public boolean onEof() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { switch (_sessionState) { @@ -181,7 +177,7 @@ public class WebSocketSessionState byte opcode = frame.getOpCode(); boolean fin = frame.isFin(); - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (!isOutputOpen()) throw new ClosedChannelException(); @@ -224,7 +220,7 @@ public class WebSocketSessionState byte opcode = frame.getOpCode(); boolean fin = frame.isFin(); - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (!isInputOpen()) throw new IllegalStateException(_sessionState.toString()); diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java index 4fe629afd7c..d1a9aa5c093 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java @@ -37,6 +37,7 @@ import javax.websocket.server.ServerEndpointConfig; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.websocket.core.CloseStatus; import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.Frame; @@ -60,55 +61,25 @@ import org.slf4j.LoggerFactory; public class JavaxWebSocketFrameHandler implements FrameHandler { + private final AutoLock lock = new AutoLock(); + private final AtomicBoolean closeNotified = new AtomicBoolean(); + private final Map messageHandlerMap = new HashMap<>(); private final Logger logger; private final JavaxWebSocketContainer container; private final Object endpointInstance; - private final AtomicBoolean closeNotified = new AtomicBoolean(); - - /** - * List of configured named variables in the uri-template. - *

- * Used to bind uri-template variables, with their values from the upgrade, to the methods - * that have declared their interest in these values via {@code @PathParam} annotations. - *

- *

- * Can be null if client side, or no named variables were configured on the server side. - *

- */ - /** - * The Map of path parameter values that arrived during the server side upgrade process. - *

- * Used to bind uri-template variables, with their values from the upgrade, to the methods - * that have declared their interest in these values via {@code @PathParam} annotations. - *

- *

- * The values are represented as {@link String} and are essentially static for this - * instance of the the JavaxWebSocketFrameHandler. They will be converted to the - * type declared by the {@code @PathParam} annotations following the JSR356 advice - * to only support String, Java Primitives (or their Boxed version). - *

- *

- * Can be null if client side, or no named variables were configured on the server side, - * or the server side component didn't use the {@link org.eclipse.jetty.http.pathmap.UriTemplatePathSpec} for its mapping. - *

- */ private MethodHandle openHandle; private MethodHandle closeHandle; private MethodHandle errorHandle; private MethodHandle pongHandle; private JavaxWebSocketMessageMetadata textMetadata; private JavaxWebSocketMessageMetadata binaryMetadata; - private UpgradeRequest upgradeRequest; - private EndpointConfig endpointConfig; - private final Map messageHandlerMap = new HashMap<>(); private MessageSink textSink; private MessageSink binarySink; private MessageSink activeMessageSink; private JavaxWebSocketSession session; private CoreSession coreSession; - protected byte dataType = OpCode.UNDEFINED; public JavaxWebSocketFrameHandler(JavaxWebSocketContainer container, @@ -526,7 +497,7 @@ public class JavaxWebSocketFrameHandler implements FrameHandler private MessageSink registerMessageHandler(byte basicWebSocketMessageType, Class handlerType, MessageHandler handler, MessageSink messageSink) { - synchronized (messageHandlerMap) + try (AutoLock ignored = lock.lock()) { RegisteredMessageHandler registeredHandler = messageHandlerMap.get(basicWebSocketMessageType); if (registeredHandler != null) @@ -546,7 +517,7 @@ public class JavaxWebSocketFrameHandler implements FrameHandler public void removeMessageHandler(MessageHandler handler) { - synchronized (messageHandlerMap) + try (AutoLock ignored = lock.lock()) { Optional> optionalEntry = messageHandlerMap.entrySet().stream() .filter((entry) -> entry.getValue().getMessageHandler().equals(handler)) diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketSession.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketSession.java index 526a7ff97bf..13b4701fd2b 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketSession.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketSession.java @@ -565,7 +565,7 @@ public class JavaxWebSocketSession implements javax.websocket.Session } @Override - public synchronized void removeMessageHandler(MessageHandler handler) + public void removeMessageHandler(MessageHandler handler) { frameHandler.removeMessageHandler(handler); } 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 dc5588d37c2..da4760d655f 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 @@ -375,7 +375,7 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketPoli * @see Runtime#addShutdownHook(Thread) * @see ShutdownThread */ - public synchronized void setStopAtShutdown(boolean stop) + public void setStopAtShutdown(boolean stop) { if (stop) { diff --git a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java index c2b8c5bee96..ae0d2675405 100644 --- a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java +++ b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java @@ -26,6 +26,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.websocket.api.BatchMode; import org.eclipse.jetty.websocket.api.UpgradeRequest; import org.eclipse.jetty.websocket.api.UpgradeResponse; @@ -60,6 +61,7 @@ public class JettyWebSocketFrameHandler implements FrameHandler CLOSED } + private final AutoLock lock = new AutoLock(); private final Logger log; private final WebSocketContainer container; private final Object endpointInstance; @@ -189,7 +191,7 @@ public class JettyWebSocketFrameHandler implements FrameHandler @Override public void onFrame(Frame frame, Callback callback) { - synchronized (this) + try (AutoLock ignored = lock.lock()) { switch (state) { @@ -298,11 +300,8 @@ public class JettyWebSocketFrameHandler implements FrameHandler @Override public void onClosed(CloseStatus closeStatus, Callback callback) { - synchronized (this) - { - // We are now closed and cannot suspend or resume. - state = SuspendState.CLOSED; - } + // We are now closed and cannot suspend or resume. + lock.runLocked(() -> state = SuspendState.CLOSED); notifyOnClose(closeStatus, callback); container.notifySessionListeners((listener) -> listener.onWebSocketSessionClosed(session)); @@ -425,7 +424,7 @@ public class JettyWebSocketFrameHandler implements FrameHandler public void suspend() { - synchronized (this) + try (AutoLock ignored = lock.lock()) { switch (state) { @@ -443,7 +442,7 @@ public class JettyWebSocketFrameHandler implements FrameHandler { boolean needDemand = false; Runnable delayedFrame = null; - synchronized (this) + try (AutoLock ignored = lock.lock()) { switch (state) { @@ -480,7 +479,7 @@ public class JettyWebSocketFrameHandler implements FrameHandler private void demand() { boolean demand = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { switch (state) { diff --git a/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/messages/MessageInputStream.java b/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/messages/MessageInputStream.java index 54ed65b55bd..f92295a0cef 100644 --- a/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/messages/MessageInputStream.java +++ b/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/messages/MessageInputStream.java @@ -29,6 +29,7 @@ import java.util.concurrent.TimeUnit; import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.websocket.core.Frame; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,6 +45,8 @@ public class MessageInputStream extends InputStream implements MessageSink private static final Logger LOG = LoggerFactory.getLogger(MessageInputStream.class); private static final Entry EOF = new Entry(BufferUtil.EMPTY_BUFFER, Callback.NOOP); private static final Entry CLOSED = new Entry(BufferUtil.EMPTY_BUFFER, Callback.NOOP); + + private final AutoLock lock = new AutoLock(); private final BlockingArrayQueue buffers = new BlockingArrayQueue<>(); private boolean closed = false; private Entry currentEntry; @@ -56,7 +59,7 @@ public class MessageInputStream extends InputStream implements MessageSink LOG.debug("accepting {}", frame); boolean succeed = false; - synchronized (this) + try (AutoLock ignored = lock.lock()) { // If closed or we have no payload, request the next frame. if (closed || (!frame.hasPayload() && !frame.isFin())) @@ -134,7 +137,7 @@ public class MessageInputStream extends InputStream implements MessageSink LOG.debug("close()"); ArrayList entries = new ArrayList<>(); - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (closed) return; @@ -169,7 +172,7 @@ public class MessageInputStream extends InputStream implements MessageSink private void succeedCurrentEntry() { Entry current; - synchronized (this) + try (AutoLock ignored = lock.lock()) { current = currentEntry; currentEntry = null; @@ -180,7 +183,7 @@ public class MessageInputStream extends InputStream implements MessageSink private Entry getCurrentEntry() throws IOException { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (currentEntry != null) return currentEntry; @@ -205,7 +208,7 @@ public class MessageInputStream extends InputStream implements MessageSink throw new IOException(String.format("Read timeout: %,dms expired", timeoutMs)); } - synchronized (this) + try (AutoLock ignored = lock.lock()) { currentEntry = result; return currentEntry; diff --git a/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/messages/MessageOutputStream.java b/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/messages/MessageOutputStream.java index 389fd20eaf5..ab7087d4b46 100644 --- a/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/messages/MessageOutputStream.java +++ b/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/messages/MessageOutputStream.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FutureCallback; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.Frame; import org.eclipse.jetty.websocket.core.OpCode; @@ -39,6 +40,7 @@ public class MessageOutputStream extends OutputStream { private static final Logger LOG = LoggerFactory.getLogger(MessageOutputStream.class); + private final AutoLock lock = new AutoLock(); private final CoreSession coreSession; private final ByteBufferPool bufferPool; private final int bufferSize; @@ -123,7 +125,7 @@ public class MessageOutputStream extends OutputStream private void flush(boolean fin) throws IOException { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (closed) throw new IOException("Stream is closed"); @@ -157,7 +159,7 @@ public class MessageOutputStream extends OutputStream private void send(ByteBuffer data) throws IOException { - synchronized (this) + try (AutoLock ignored = lock.lock()) { if (closed) throw new IOException("Stream is closed"); @@ -197,35 +199,20 @@ public class MessageOutputStream extends OutputStream public void setCallback(Callback callback) { - synchronized (this) - { - this.callback = callback; - } + lock.runLocked(() -> this.callback = callback); } private void notifySuccess() { - Callback callback; - synchronized (this) - { - callback = this.callback; - } + Callback callback = lock.runLocked(() -> this.callback); if (callback != null) - { callback.succeeded(); - } } private void notifyFailure(Throwable failure) { - Callback callback; - synchronized (this) - { - callback = this.callback; - } + Callback callback = lock.runLocked(() -> this.callback); if (callback != null) - { callback.failed(failure); - } } } diff --git a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java index c3cca7ec114..d11bb162b49 100644 --- a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java +++ b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlConfiguration.java @@ -63,6 +63,7 @@ import org.eclipse.jetty.util.TypeUtil; import org.eclipse.jetty.util.annotation.Name; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.resource.Resource; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xml.sax.SAXException; @@ -249,7 +250,7 @@ public class XmlConfiguration */ public XmlConfiguration(Resource resource) throws SAXException, IOException { - synchronized (PARSER) + try (AutoLock ignored = PARSER.lock()) { _location = resource; try (InputStream inputStream = resource.getInputStream()) diff --git a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlParser.java b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlParser.java index 58f8da3c488..007ab207b9c 100644 --- a/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlParser.java +++ b/jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlParser.java @@ -35,6 +35,7 @@ import javax.xml.parsers.SAXParserFactory; import org.eclipse.jetty.util.LazyList; import org.eclipse.jetty.util.resource.Resource; +import org.eclipse.jetty.util.thread.AutoLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xml.sax.Attributes; @@ -56,17 +57,15 @@ public class XmlParser { private static final Logger LOG = LoggerFactory.getLogger(XmlParser.class); - private Map _redirectMap = new HashMap(); + private final AutoLock _lock = new AutoLock(); + private final Map _redirectMap = new HashMap<>(); + private final Stack _observers = new Stack<>(); private SAXParser _parser; private Map _observerMap; - private Stack _observers = new Stack(); private String _xpath; private Object _xpaths; private String _dtd; - /** - * Construct - */ public XmlParser() { SAXParserFactory factory = SAXParserFactory.newInstance(); @@ -81,6 +80,11 @@ public class XmlParser setValidating(validating); } + AutoLock lock() + { + return _lock.lock(); + } + public void setValidating(boolean validating) { try @@ -127,10 +131,10 @@ public class XmlParser return _parser.isValidating(); } - public synchronized void redirectEntity(String name, URL entity) + public void redirectEntity(String name, URL entity) { if (entity != null) - _redirectMap.put(name, entity); + _lock.runLocked(() -> _redirectMap.put(name, entity)); } /** @@ -170,29 +174,35 @@ public class XmlParser * @param trigger Tag local or q name. * @param observer SAX ContentHandler */ - public synchronized void addContentHandler(String trigger, ContentHandler observer) + public void addContentHandler(String trigger, ContentHandler observer) { - if (_observerMap == null) - _observerMap = new HashMap<>(); - _observerMap.put(trigger, observer); + try (AutoLock ignored = _lock.lock()) + { + if (_observerMap == null) + _observerMap = new HashMap<>(); + _observerMap.put(trigger, observer); + } } - public synchronized Node parse(InputSource source) throws IOException, SAXException + public Node parse(InputSource source) throws IOException, SAXException { - _dtd = null; - Handler handler = new Handler(); - XMLReader reader = _parser.getXMLReader(); - reader.setContentHandler(handler); - reader.setErrorHandler(handler); - reader.setEntityResolver(handler); - if (LOG.isDebugEnabled()) - LOG.debug("parsing: sid=" + source.getSystemId() + ",pid=" + source.getPublicId()); - _parser.parse(source, handler); - if (handler._error != null) - throw handler._error; - Node doc = (Node)handler._top.get(0); - handler.clear(); - return doc; + try (AutoLock ignored = _lock.lock()) + { + _dtd = null; + Handler handler = new Handler(); + XMLReader reader = _parser.getXMLReader(); + reader.setContentHandler(handler); + reader.setErrorHandler(handler); + reader.setEntityResolver(handler); + if (LOG.isDebugEnabled()) + LOG.debug("parsing: sid=" + source.getSystemId() + ",pid=" + source.getPublicId()); + _parser.parse(source, handler); + if (handler._error != null) + throw handler._error; + Node doc = (Node)handler._top.get(0); + handler.clear(); + return doc; + } } /** @@ -203,7 +213,7 @@ public class XmlParser * @throws IOException if unable to load the xml * @throws SAXException if unable to parse the xml */ - public synchronized Node parse(String url) throws IOException, SAXException + public Node parse(String url) throws IOException, SAXException { if (LOG.isDebugEnabled()) LOG.debug("parse: " + url); @@ -218,7 +228,7 @@ public class XmlParser * @throws IOException if unable to load the xml * @throws SAXException if unable to parse the xml */ - public synchronized Node parse(File file) throws IOException, SAXException + public Node parse(File file) throws IOException, SAXException { if (LOG.isDebugEnabled()) LOG.debug("parse: " + file); @@ -233,20 +243,9 @@ public class XmlParser * @throws IOException if unable to load the xml * @throws SAXException if unable to parse the xml */ - public synchronized Node parse(InputStream in) throws IOException, SAXException + public Node parse(InputStream in) throws IOException, SAXException { - _dtd = null; - Handler handler = new Handler(); - XMLReader reader = _parser.getXMLReader(); - reader.setContentHandler(handler); - reader.setErrorHandler(handler); - reader.setEntityResolver(handler); - _parser.parse(new InputSource(in), handler); - if (handler._error != null) - throw handler._error; - Node doc = (Node)handler._top.get(0); - handler.clear(); - return doc; + return parse(new InputSource(in)); } protected InputSource resolveEntity(String pid, String sid) @@ -629,7 +628,7 @@ public class XmlParser public void add(int i, Object o) { if (_list == null) - _list = new ArrayList(); + _list = new ArrayList<>(); if (o instanceof String) { if (_lastString) @@ -676,7 +675,7 @@ public class XmlParser } @Override - public synchronized String toString() + public String toString() { return toString(true); } @@ -687,7 +686,7 @@ public class XmlParser * @param tag If false, only _content is shown. * @return the string value */ - public synchronized String toString(boolean tag) + public String toString(boolean tag) { StringBuilder buf = new StringBuilder(); toString(buf, tag); @@ -701,7 +700,7 @@ public class XmlParser * @param trim true to trim the content * @return the trimmed content */ - public synchronized String toString(boolean tag, boolean trim) + public String toString(boolean tag, boolean trim) { String s = toString(tag); if (s != null && trim) @@ -709,7 +708,7 @@ public class XmlParser return s; } - private synchronized void toString(StringBuilder buf, boolean tag) + private void toString(StringBuilder buf, boolean tag) { if (tag) { diff --git a/tests/jetty-http-tools/src/test/java/org/eclipse/jetty/http/tools/HttpTesterTest.java b/tests/jetty-http-tools/src/test/java/org/eclipse/jetty/http/tools/HttpTesterTest.java index c09db1f6497..0fce50844f8 100644 --- a/tests/jetty-http-tools/src/test/java/org/eclipse/jetty/http/tools/HttpTesterTest.java +++ b/tests/jetty-http-tools/src/test/java/org/eclipse/jetty/http/tools/HttpTesterTest.java @@ -256,7 +256,7 @@ public class HttpTesterTest PipedInputStream stream = new PipedInputStream(src) { @Override - public synchronized int read(byte[] b, int off, int len) throws IOException + public int read(byte[] b, int off, int len) throws IOException { if (available() == 0) return 0; diff --git a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/util/jmh/DateCacheSimpleDateFormat.java b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/util/jmh/DateCacheSimpleDateFormat.java index 1b6e8076480..10430a84d80 100644 --- a/tests/jetty-jmh/src/main/java/org/eclipse/jetty/util/jmh/DateCacheSimpleDateFormat.java +++ b/tests/jetty-jmh/src/main/java/org/eclipse/jetty/util/jmh/DateCacheSimpleDateFormat.java @@ -23,6 +23,8 @@ import java.util.Date; import java.util.Locale; import java.util.TimeZone; +import org.eclipse.jetty.util.thread.AutoLock; + /** * Date Format Cache. * Computes String representations of Dates and caches @@ -40,14 +42,11 @@ import java.util.TimeZone; */ public class DateCacheSimpleDateFormat { - public static final String DEFAULT_FORMAT = "EEE MMM dd HH:mm:ss zzz yyyy"; + private final AutoLock _lock = new AutoLock(); private final String _formatString; - private final String _tzFormatString; private final SimpleDateFormat _tzFormat; - private final Locale _locale; - private volatile Tick _tick; public static class Tick @@ -96,9 +95,9 @@ public class DateCacheSimpleDateFormat public DateCacheSimpleDateFormat(String format, Locale l, TimeZone tz) { _formatString = format; - _locale = l; int zIndex = _formatString.indexOf("ZZZ"); + String tzFormatString; if (zIndex >= 0) { final String ss1 = _formatString.substring(0, zIndex); @@ -129,19 +128,17 @@ public class DateCacheSimpleDateFormat sb.append('\''); sb.append(ss2); - _tzFormatString = sb.toString(); + tzFormatString = sb.toString(); } else - _tzFormatString = _formatString; + { + tzFormatString = _formatString; + } - if (_locale != null) - { - _tzFormat = new SimpleDateFormat(_tzFormatString, _locale); - } + if (l != null) + _tzFormat = new SimpleDateFormat(tzFormatString, l); else - { - _tzFormat = new SimpleDateFormat(_tzFormatString); - } + _tzFormat = new SimpleDateFormat(tzFormatString); _tzFormat.setTimeZone(tz); _tick = null; @@ -168,10 +165,7 @@ public class DateCacheSimpleDateFormat if (tick == null || seconds != tick._seconds) { // It's a cache miss - synchronized (this) - { - return _tzFormat.format(inDate); - } + return _lock.runLocked(() -> _tzFormat.format(inDate)); } return tick._string; @@ -196,10 +190,7 @@ public class DateCacheSimpleDateFormat { // It's a cache miss Date d = new Date(inDate); - synchronized (this) - { - return _tzFormat.format(d); - } + return _lock.runLocked(() -> _tzFormat.format(d)); } return tick._string; @@ -241,7 +232,7 @@ public class DateCacheSimpleDateFormat long seconds = now / 1000; // Synchronize to protect _tzFormat - synchronized (this) + try (AutoLock ignored = _lock.lock()) { // recheck the tick, to save multiple formats if (_tick == null || _tick._seconds != seconds) diff --git a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/DispatchServlet.java b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/DispatchServlet.java index b8938aa1bae..cce49261271 100644 --- a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/DispatchServlet.java +++ b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/DispatchServlet.java @@ -255,9 +255,4 @@ public class DispatchServlet extends HttpServlet { return "Include Servlet"; } - - @Override - public synchronized void destroy() - { - } } diff --git a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/Dump.java b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/Dump.java index cda240ce815..8859cb69cb0 100644 --- a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/Dump.java +++ b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/Dump.java @@ -919,7 +919,7 @@ public class Dump extends HttpServlet } @Override - public synchronized void destroy() + public void destroy() { _timer.cancel(); } diff --git a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/RegTest.java b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/RegTest.java index 7db6bace9be..ca23b1ab532 100644 --- a/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/RegTest.java +++ b/tests/test-webapps/test-jetty-webapp/src/main/java/com/acme/RegTest.java @@ -164,11 +164,6 @@ public class RegTest extends HttpServlet return "Rego Servlet"; } - @Override - public synchronized void destroy() - { - } - private String notag(String s) { if (s == null)