From 692c01750057ca2cd1a54aaf6b07700364c1a62b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 9 Sep 2019 15:57:45 +0200 Subject: [PATCH] Fixes #4058 - Review Locker. Removes the Locker class, replaced by AutoLock. Removed usages of Locker.isLocked() from the session code since it was not necessary. Took the chance to do a little code cleanup. Signed-off-by: Simone Bordet --- .../eclipse/jetty/io/ByteArrayEndPoint.java | 47 ++++---- .../jetty/server/AbstractConnector.java | 22 ++-- .../java/org/eclipse/jetty/server/Server.java | 7 +- .../server/handler/ThreadLimitHandler.java | 20 ++-- .../server/session/AbstractSessionCache.java | 64 +++-------- .../eclipse/jetty/server/session/Session.java | 101 +++++++----------- .../jetty/server/session/SessionCache.java | 7 +- .../jetty/server/session/SessionHandler.java | 5 +- .../eclipse/jetty/util/IteratingCallback.java | 83 +++++++------- .../thread/{Locker.java => AutoLock.java} | 42 +++----- .../util/thread/ReservedThreadExecutor.java | 8 +- .../strategy/ExecuteProduceConsume.java | 22 ++-- .../util/thread/strategy/ProduceConsume.java | 8 +- .../strategy/ProduceExecuteConsume.java | 11 +- .../{LockerTest.java => AutoLockTest.java} | 60 +++++------ .../jetty/server/session/IdleSessionTest.java | 24 +++-- 16 files changed, 215 insertions(+), 316 deletions(-) rename jetty-util/src/main/java/org/eclipse/jetty/util/thread/{Locker.java => AutoLock.java} (68%) rename jetty-util/src/test/java/org/eclipse/jetty/util/thread/{LockerTest.java => AutoLockTest.java} (71%) 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 14948563718..cdfd2fac200 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 @@ -36,7 +36,7 @@ import java.util.concurrent.locks.Condition; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.thread.Locker; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.Scheduler; /** @@ -68,24 +68,13 @@ public class ByteArrayEndPoint extends AbstractEndPoint private static final ByteBuffer EOF = BufferUtil.allocate(0); - private final Runnable _runFillable = new Runnable() - { - @Override - public void run() - { - getFillInterest().fillable(); - } - }; - - private final Locker _locker = new Locker(); - private final Condition _hasOutput = _locker.newCondition(); + private final Runnable _runFillable = () -> getFillInterest().fillable(); + private final AutoLock _lock = new AutoLock(); + private final Condition _hasOutput = _lock.newCondition(); private final Queue _inQ = new ArrayDeque<>(); private ByteBuffer _out; private boolean _growOutput; - /** - * - */ public ByteArrayEndPoint() { this(null, 0, null, null); @@ -138,7 +127,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint public void doShutdownOutput() { super.doShutdownOutput(); - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { _hasOutput.signalAll(); } @@ -148,7 +137,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint public void doClose() { super.doClose(); - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { _hasOutput.signalAll(); } @@ -180,7 +169,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint @Override protected void needsFillInterest() throws IOException { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { if (!isOpen()) throw new ClosedChannelException(); @@ -205,7 +194,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint public void addInput(ByteBuffer in) { boolean fillable = false; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { if (isEOF(_inQ.peek())) throw new RuntimeIOException(new EOFException()); @@ -238,7 +227,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint public void addInputAndExecute(ByteBuffer in) { boolean fillable = false; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { if (isEOF(_inQ.peek())) throw new RuntimeIOException(new EOFException()); @@ -263,7 +252,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint */ public ByteBuffer getOutput() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { return _out; } @@ -293,7 +282,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint { ByteBuffer b; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { b = _out; _out = BufferUtil.allocate(b.capacity()); @@ -314,7 +303,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint { ByteBuffer b; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { while (BufferUtil.isEmpty(_out) && !isOutputShutdown()) { @@ -351,7 +340,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint */ public void setOutput(ByteBuffer out) { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { _out = out; } @@ -359,7 +348,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint } /** - * @return true if there are bytes remaining to be read from the encoded input + * @return {@code true} if there are bytes remaining to be read from the encoded input */ public boolean hasMore() { @@ -373,7 +362,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint public int fill(ByteBuffer buffer) throws IOException { int filled = 0; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { while (true) { @@ -418,7 +407,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint public boolean flush(ByteBuffer... buffers) throws IOException { boolean flushed = true; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { if (!isOpen()) throw new IOException("CLOSED"); @@ -467,7 +456,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint @Override public void reset() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { _inQ.clear(); _hasOutput.signalAll(); @@ -507,7 +496,7 @@ public class ByteArrayEndPoint extends AbstractEndPoint int q; ByteBuffer b; String o; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { q = _inQ.size(); b = _inQ.peek(); 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 1723bcc73df..d076742d121 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 @@ -52,7 +52,7 @@ import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.ssl.SslContextFactory; -import org.eclipse.jetty.util.thread.Locker; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; import org.eclipse.jetty.util.thread.Scheduler; import org.eclipse.jetty.util.thread.ThreadPoolBudget; @@ -144,8 +144,8 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co { protected static final Logger LOG = Log.getLogger(AbstractConnector.class); - private final Locker _locker = new Locker(); - private final Condition _setAccepting = _locker.newCondition(); + private final AutoLock _lock = new AutoLock(); + private final Condition _setAccepting = _lock.newCondition(); 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; @@ -231,7 +231,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co * Get the {@link HttpChannel.Listener}s added to the connector * as a single combined Listener. * This is equivalent to a listener that iterates over the individual - * listeners returned from getBeans(HttpChannel.Listener.class);, + * listeners returned from {@code getBeans(HttpChannel.Listener.class);}, * except that:
    *
  • The result is precomputed, so it is more efficient
    • *
  • The result is ordered by the order added.
    • @@ -332,7 +332,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co protected void interruptAcceptors() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { for (Thread thread : _acceptors) { @@ -387,7 +387,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co public void join(long timeout) throws InterruptedException { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { for (Thread thread : _acceptors) { @@ -404,7 +404,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co */ public boolean isAccepting() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { return _accepting; } @@ -412,7 +412,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co public void setAccepting(boolean accepting) { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { _accepting = accepting; _setAccepting.signalAll(); @@ -422,7 +422,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co @Override public ConnectionFactory getConnectionFactory(String protocol) { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { return _factories.get(StringUtil.asciiToLowerCase(protocol)); } @@ -431,7 +431,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co @Override public T getConnectionFactory(Class factoryType) { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { for (ConnectionFactory f : _factories.values()) { @@ -683,7 +683,7 @@ public abstract class AbstractConnector extends ContainerLifeCycle implements Co { while (isRunning()) { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { if (!_accepting && isRunning()) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 282baf968c1..4618a8edf2f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -57,7 +57,7 @@ import org.eclipse.jetty.util.component.AttributeContainerMap; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.thread.Locker; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.util.thread.ShutdownThread; import org.eclipse.jetty.util.thread.ThreadPool; @@ -83,8 +83,7 @@ public class Server extends HandlerWrapper implements Attributes private boolean _dumpBeforeStop = false; private ErrorHandler _errorHandler; private RequestLog _requestLog; - - private final Locker _dateLocker = new Locker(); + private final AutoLock _dateLock = new AutoLock(); private volatile DateField _dateField; public Server() @@ -315,7 +314,7 @@ public class Server extends HandlerWrapper implements Attributes if (df == null || df._seconds != seconds) { - try (Locker.Lock lock = _dateLocker.lock()) + try (AutoLock lock = _dateLock.lock()) { df = _dateField; if (df == null || df._seconds != seconds) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ThreadLimitHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ThreadLimitHandler.java index 7666bcb0641..feab2f5644a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ThreadLimitHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ThreadLimitHandler.java @@ -48,7 +48,7 @@ import org.eclipse.jetty.util.annotation.ManagedOperation; import org.eclipse.jetty.util.annotation.Name; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.thread.Locker; +import org.eclipse.jetty.util.thread.AutoLock; /** *

    Handler to limit the threads per IP address for DOS protection

    @@ -241,7 +241,7 @@ public class ThreadLimitHandler extends HandlerWrapper } } - protected Remote getRemote(Request baseRequest) + private Remote getRemote(Request baseRequest) { Remote remote = (Remote)baseRequest.getAttribute(REMOTE); if (remote != null) @@ -329,11 +329,11 @@ public class ThreadLimitHandler extends HandlerWrapper return (comma >= 0) ? forwardedFor.substring(comma + 1).trim() : forwardedFor; } - private final class Remote implements Closeable + private static final class Remote implements Closeable { private final String _ip; private final int _limit; - private final Locker _locker = new Locker(); + private final AutoLock _lock = new AutoLock(); private int _permits; private Deque> _queue = new ArrayDeque<>(); private final CompletableFuture _permitted = CompletableFuture.completedFuture(this); @@ -346,7 +346,7 @@ public class ThreadLimitHandler extends HandlerWrapper public CompletableFuture acquire() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { // Do we have available passes? if (_permits < _limit) @@ -358,16 +358,16 @@ public class ThreadLimitHandler extends HandlerWrapper } // No pass available, so queue a new future - CompletableFuture pass = new CompletableFuture(); + CompletableFuture pass = new CompletableFuture<>(); _queue.addLast(pass); return pass; } } @Override - public void close() throws IOException + public void close() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { // reduce the allocated passes _permits--; @@ -396,14 +396,14 @@ public class ThreadLimitHandler extends HandlerWrapper @Override public String toString() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { return String.format("R[ip=%s,p=%d,l=%d,q=%d]", _ip, _permits, _limit, _queue.size()); } } } - private final class RFC7239 extends QuotedCSV + private static final class RFC7239 extends QuotedCSV { String _for; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java index 11b1ec7453b..8cac60600cc 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java @@ -29,7 +29,7 @@ import org.eclipse.jetty.util.annotation.ManagedObject; import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.thread.Locker.Lock; +import org.eclipse.jetty.util.thread.AutoLock; /** * AbstractSessionCache @@ -110,9 +110,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements */ public abstract Session newSession(HttpServletRequest request, SessionData data); - /** - * @see org.eclipse.jetty.server.session.SessionCache#newSession(javax.servlet.http.HttpServletRequest, java.lang.String, long, long) - */ @Override public Session newSession(HttpServletRequest request, String id, long time, long maxInactiveMs) { @@ -169,7 +166,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements /** * PlaceHolder */ - protected class PlaceHolderSession extends Session + protected static class PlaceHolderSession extends Session { /** @@ -199,9 +196,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements return _handler; } - /** - * @see org.eclipse.jetty.server.session.SessionCache#initialize(org.eclipse.jetty.server.session.SessionContext) - */ @Override public void initialize(SessionContext context) { @@ -248,9 +242,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements return _sessionDataStore; } - /** - * @see org.eclipse.jetty.server.session.SessionCache#setSessionDataStore(org.eclipse.jetty.server.session.SessionDataStore) - */ @Override public void setSessionDataStore(SessionDataStore sessionStore) { @@ -258,9 +249,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements _sessionDataStore = sessionStore; } - /** - * @see org.eclipse.jetty.server.session.SessionCache#getEvictionPolicy() - */ @ManagedAttribute(value = "session eviction policy", readonly = true) @Override public int getEvictionPolicy() @@ -272,8 +260,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements * -1 means we never evict inactive sessions. * 0 means we evict a session after the last request for it exits * >0 is the number of seconds after which we evict inactive sessions from the cache - * - * @see org.eclipse.jetty.server.session.SessionCache#setEvictionPolicy(int) */ @Override public void setEvictionPolicy(int evictionTimeout) @@ -308,7 +294,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements * If a session's data cannot be loaded from the store without error, remove * it from the persistent store. * - * @param removeUnloadableSessions if true unloadable sessions will be removed from session store + * @param removeUnloadableSessions if {@code true} unloadable sessions will be removed from session store */ @Override public void setRemoveUnloadableSessions(boolean removeUnloadableSessions) @@ -322,8 +308,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements * If the session object is not in this session store, try getting * the data for it from a SessionDataStore associated with the * session manager. The usage count of the session is incremented. - * - * @see org.eclipse.jetty.server.session.SessionCache#get(java.lang.String) */ @Override public Session get(String id) throws Exception @@ -339,8 +323,8 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements * * @param id The session to retrieve * @param enter if true, the usage count of the session will be incremented - * @return - * @throws Exception + * @return the Session object + * @throws Exception if the session cannot be retrieved */ protected Session getAndEnter(String id, boolean enter) throws Exception { @@ -361,7 +345,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements //didn't get a session, try and create one and put in a placeholder for it PlaceHolderSession phs = new PlaceHolderSession(_handler, new SessionData(id, null, null, 0, 0, 0, 0)); - Lock phsLock = phs.lock(); + AutoLock phsLock = phs.lock(); Session s = doPutIfAbsent(id, phs); if (s == null) { @@ -377,7 +361,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements break; } - try (Lock lock = session.lock()) + try (AutoLock lock = session.lock()) { //swap it in instead of the placeholder boolean success = doReplace(id, phs, session); @@ -414,7 +398,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements { //my placeholder didn't win, check the session returned phsLock.close(); - try (Lock lock = s.lock()) + try (AutoLock lock = s.lock()) { //is it a placeholder? or is a non-resident session? In both cases, chuck it away and start again if (!s.isResident() || s instanceof PlaceHolderSession) @@ -433,7 +417,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements else { //check the session returned - try (Lock lock = session.lock()) + try (AutoLock lock = session.lock()) { //is it a placeholder? or is it passivated? In both cases, chuck it away and start again if (!session.isResident() || session instanceof PlaceHolderSession) @@ -494,8 +478,8 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements * Add an entirely new session (created by the application calling Request.getSession(true)) * to the cache. The usage count of the fresh session is incremented. * - * @param id the id - * @param session + * @param id the session id + * @param session the new session to add */ @Override public void add(String id, Session session) throws Exception @@ -503,7 +487,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements if (id == null || session == null) throw new IllegalArgumentException("Add key=" + id + " session=" + (session == null ? "null" : session.getId())); - try (Lock lock = session.lock()) + try (AutoLock lock = session.lock()) { if (session.getSessionHandler() == null) throw new IllegalStateException("Session " + id + " is not managed"); @@ -521,9 +505,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements } } - /** - * @deprecated - */ @Override public void put(String id, Session session) throws Exception { @@ -542,8 +523,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements * * If the evictionPolicy == SessionCache.EVICT_ON_SESSION_EXIT then after we have saved * the session, we evict it from the cache. - * - * @see org.eclipse.jetty.server.session.SessionCache#release(java.lang.String, org.eclipse.jetty.server.session.Session) */ @Override public void release(String id, Session session) throws Exception @@ -551,7 +530,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements if (id == null || session == null) throw new IllegalArgumentException("Put key=" + id + " session=" + (session == null ? "null" : session.getId())); - try (Lock lock = session.lock()) + try (AutoLock lock = session.lock()) { if (session.getSessionHandler() == null) throw new IllegalStateException("Session " + id + " is not managed"); @@ -630,7 +609,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements * it will check with the data store. * * @throws Exception the Exception - * @see org.eclipse.jetty.server.session.SessionCache#exists(java.lang.String) */ @Override public boolean exists(String id) throws Exception @@ -639,7 +617,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements Session s = doGet(id); if (s != null) { - try (Lock lock = s.lock()) + try (AutoLock lock = s.lock()) { //wait for the lock and check the validity of the session return s.isValid(); @@ -653,8 +631,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements /** * Check to see if this cache contains an entry for the session * corresponding to the session id. - * - * @see org.eclipse.jetty.server.session.SessionCache#contains(java.lang.String) */ @Override public boolean contains(String id) throws Exception @@ -665,8 +641,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements /** * Remove a session object from this store and from any backing store. - * - * @see org.eclipse.jetty.server.session.SessionCache#delete(java.lang.String) */ @Override public Session delete(String id) throws Exception @@ -691,9 +665,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements return doDelete(id); } - /** - * @see org.eclipse.jetty.server.session.SessionCache#checkExpiration(Set) - */ @Override public Set checkExpiration(Set candidates) { @@ -741,7 +712,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements if (LOG.isDebugEnabled()) LOG.debug("Checking for idle {}", session.getId()); - try (Lock s = session.lock()) + try (AutoLock lock = session.lock()) { if (getEvictionPolicy() > 0 && session.isIdleLongerThan(getEvictionPolicy()) && session.isValid() && session.isResident() && session.getRequests() <= 0) @@ -803,7 +774,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements if (session == null) return; - try (Lock lock = session.lock()) + try (AutoLock lock = session.lock()) { final String oldId = session.getId(); session.checkValidForWrite(); //can't change id on invalid session @@ -826,9 +797,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements } } - /** - * @see org.eclipse.jetty.server.session.SessionCache#setSaveOnInactiveEviction(boolean) - */ @Override public void setSaveOnInactiveEviction(boolean saveOnEvict) { 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 f33b4d83f1f..8af585f4059 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 @@ -36,8 +36,7 @@ import javax.servlet.http.HttpSessionEvent; import org.eclipse.jetty.io.CyclicTimeout; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.thread.Locker; -import org.eclipse.jetty.util.thread.Locker.Lock; +import org.eclipse.jetty.util.thread.AutoLock; /** * Session @@ -73,15 +72,11 @@ public class Session implements SessionHandler.SessionIf VALID, INVALID, INVALIDATING, CHANGING } - ; - public enum IdState { SET, CHANGING } - ; - protected final SessionData _sessionData; // the actual data associated with // a session @@ -98,7 +93,7 @@ public class Session implements SessionHandler.SessionIf protected State _state = State.VALID; // state of the session:valid,invalid // or being invalidated - protected Locker _lock = new Locker(); // sync lock + protected AutoLock _lock = new AutoLock(); protected Condition _stateChangeCompleted = _lock.newCondition(); protected boolean _resident = false; protected final SessionInactivityTimer _sessionInactivityTimer; @@ -128,7 +123,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 (Lock lock = Session.this.lock()) + try (AutoLock lock = 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 @@ -213,7 +208,7 @@ public class Session implements SessionHandler.SessionIf */ public long getRequests() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { return _requests; } @@ -226,7 +221,7 @@ public class Session implements SessionHandler.SessionIf protected void cookieSet() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { _sessionData.setCookieSet(_sessionData.getAccessed()); } @@ -234,7 +229,7 @@ public class Session implements SessionHandler.SessionIf protected void use() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { _requests++; @@ -247,7 +242,7 @@ public class Session implements SessionHandler.SessionIf protected boolean access(long time) { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { if (!isValid() || !isResident()) return false; @@ -267,7 +262,7 @@ public class Session implements SessionHandler.SessionIf protected void complete() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { _requests--; @@ -294,7 +289,7 @@ public class Session implements SessionHandler.SessionIf */ protected boolean isExpiredAt(long time) { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { return _sessionData.isExpiredAt(time); } @@ -309,7 +304,7 @@ public class Session implements SessionHandler.SessionIf protected boolean isIdleLongerThan(int sec) { long now = System.currentTimeMillis(); - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { return ((_sessionData.getAccessed() + (sec * 1000)) <= now); } @@ -350,7 +345,7 @@ public class Session implements SessionHandler.SessionIf */ public void unbindValue(java.lang.String name, Object value) { - if (value != null && value instanceof HttpSessionBindingListener) + if (value instanceof HttpSessionBindingListener) ((HttpSessionBindingListener)value).valueUnbound(new HttpSessionBindingEvent(this, name)); } @@ -363,7 +358,7 @@ public class Session implements SessionHandler.SessionIf */ public void bindValue(java.lang.String name, Object value) { - if (value != null && value instanceof HttpSessionBindingListener) + if (value instanceof HttpSessionBindingListener) ((HttpSessionBindingListener)value).valueBound(new HttpSessionBindingEvent(this, name)); } @@ -373,9 +368,9 @@ public class Session implements SessionHandler.SessionIf public void didActivate() { HttpSessionEvent event = new HttpSessionEvent(this); - for (Iterator iter = _sessionData.getKeys().iterator(); iter.hasNext();) + for (String name : _sessionData.getKeys()) { - Object value = _sessionData.getAttribute(iter.next()); + Object value = _sessionData.getAttribute(name); if (value instanceof HttpSessionActivationListener) { HttpSessionActivationListener listener = (HttpSessionActivationListener)value; @@ -390,9 +385,9 @@ public class Session implements SessionHandler.SessionIf public void willPassivate() { HttpSessionEvent event = new HttpSessionEvent(this); - for (Iterator iter = _sessionData.getKeys().iterator(); iter.hasNext();) + for (String name : _sessionData.getKeys()) { - Object value = _sessionData.getAttribute(iter.next()); + Object value = _sessionData.getAttribute(name); if (value instanceof HttpSessionActivationListener) { HttpSessionActivationListener listener = (HttpSessionActivationListener)value; @@ -403,7 +398,7 @@ public class Session implements SessionHandler.SessionIf public boolean isValid() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { return _state == State.VALID; } @@ -411,21 +406,15 @@ public class Session implements SessionHandler.SessionIf public boolean isInvalid() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { return _state == State.INVALID || _state == State.INVALIDATING; } } - public boolean isChanging() - { - checkLocked(); - return _state == State.CHANGING; - } - public long getCookieSetTime() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { return _sessionData.getCookieSet(); } @@ -434,7 +423,7 @@ public class Session implements SessionHandler.SessionIf @Override public long getCreationTime() throws IllegalStateException { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { checkValidForRead(); return _sessionData.getCreated(); @@ -447,7 +436,7 @@ public class Session implements SessionHandler.SessionIf @Override public String getId() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { return _sessionData.getId(); } @@ -474,7 +463,7 @@ public class Session implements SessionHandler.SessionIf @Override public long getLastAccessedTime() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { if (isInvalid()) { @@ -501,7 +490,7 @@ public class Session implements SessionHandler.SessionIf @Override public void setMaxInactiveInterval(int secs) { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { _sessionData.setMaxInactiveMs((long)secs * 1000L); _sessionData.calcAndSetExpiry(); @@ -530,7 +519,7 @@ public class Session implements SessionHandler.SessionIf { long time = 0; - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { long remaining = _sessionData.getExpiry() - now; long maxInactive = _sessionData.getMaxInactiveMs(); @@ -594,7 +583,7 @@ public class Session implements SessionHandler.SessionIf @Override public int getMaxInactiveInterval() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { long maxInactiveMs = _sessionData.getMaxInactiveMs(); return (int)(maxInactiveMs < 0 ? -1 : maxInactiveMs / 1000); @@ -624,8 +613,6 @@ public class Session implements SessionHandler.SessionIf */ protected void checkValidForWrite() throws IllegalStateException { - checkLocked(); - if (_state == State.INVALID) throw new IllegalStateException("Not valid for write: id=" + _sessionData.getId() + " created=" + _sessionData.getCreated() + @@ -649,8 +636,6 @@ public class Session implements SessionHandler.SessionIf */ protected void checkValidForRead() throws IllegalStateException { - checkLocked(); - if (_state == State.INVALID) throw new IllegalStateException("Invalid for read: id=" + _sessionData.getId() + " created=" + _sessionData.getCreated() + @@ -666,19 +651,13 @@ public class Session implements SessionHandler.SessionIf throw new IllegalStateException("Invalid for read: id=" + _sessionData.getId() + " not resident"); } - protected void checkLocked() throws IllegalStateException - { - if (!_lock.isLocked()) - throw new IllegalStateException("Session not locked"); - } - /** * @see javax.servlet.http.HttpSession#getAttribute(java.lang.String) */ @Override public Object getAttribute(String name) { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { checkValidForRead(); return _sessionData.getAttribute(name); @@ -692,7 +671,7 @@ public class Session implements SessionHandler.SessionIf @Deprecated(since = "Servlet API 2.2") public Object getValue(String name) { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { checkValidForRead(); return _sessionData.getAttribute(name); @@ -705,11 +684,11 @@ public class Session implements SessionHandler.SessionIf @Override public Enumeration getAttributeNames() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { checkValidForRead(); final Iterator itor = _sessionData.getKeys().iterator(); - return new Enumeration() + return new Enumeration<>() { @Override @@ -745,7 +724,7 @@ public class Session implements SessionHandler.SessionIf @Deprecated(since = "Servlet API 2.2") public String[] getValueNames() throws IllegalStateException { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { checkValidForRead(); Iterator itor = _sessionData.getKeys().iterator(); @@ -768,7 +747,7 @@ public class Session implements SessionHandler.SessionIf public void setAttribute(String name, Object value) { Object old = null; - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { // if session is not valid, don't accept the set checkValidForWrite(); @@ -822,7 +801,7 @@ public class Session implements SessionHandler.SessionIf String id = null; String extendedId = null; - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { while (true) { @@ -858,7 +837,7 @@ public class Session implements SessionHandler.SessionIf String newId = _handler._sessionIdManager.renewSessionId(id, extendedId, request); - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { @@ -936,7 +915,7 @@ public class Session implements SessionHandler.SessionIf * * @return the lock */ - public Lock lock() + public AutoLock lock() { return _lock.lock(); } @@ -948,7 +927,7 @@ public class Session implements SessionHandler.SessionIf { boolean result = false; - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { while (true) @@ -1007,7 +986,7 @@ public class Session implements SessionHandler.SessionIf */ protected void finishInvalidate() throws IllegalStateException { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { try { @@ -1045,7 +1024,7 @@ public class Session implements SessionHandler.SessionIf @Override public boolean isNew() throws IllegalStateException { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { checkValidForRead(); return _newSession; @@ -1054,7 +1033,7 @@ public class Session implements SessionHandler.SessionIf public void setIdChanged(boolean changed) { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { _idChanged = changed; } @@ -1062,7 +1041,7 @@ public class Session implements SessionHandler.SessionIf public boolean isIdChanged() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _lock.lock()) { return _idChanged; } @@ -1099,7 +1078,7 @@ public class Session implements SessionHandler.SessionIf @Override public String toString() { - try (Lock lock = _lock.lock()) + try (AutoLock lock = _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/SessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java index 69bcceeafa1..a9b38f95651 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java @@ -128,9 +128,9 @@ public interface SessionCache extends LifeCycle * @throws Exception if any error occurred * @deprecated @see release */ + @Deprecated void put(String id, Session session) throws Exception; - - + /** * Finish using a Session. This is called by the SessionHandler * once a request is finished with a Session. SessionCache @@ -142,8 +142,7 @@ public interface SessionCache extends LifeCycle * @throws Exception if any error occurred */ void release(String id, Session session) throws Exception; - - + /** * Check to see if a Session is in the cache. Does NOT consult * the SessionDataStore. 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 4f4dc81d758..0d6e2c48a7f 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 @@ -29,7 +29,6 @@ import java.util.List; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; - import javax.servlet.DispatcherType; import javax.servlet.ServletException; import javax.servlet.SessionCookieConfig; @@ -59,7 +58,7 @@ import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.statistic.CounterStatistic; import org.eclipse.jetty.util.statistic.SampleStatistic; -import org.eclipse.jetty.util.thread.Locker.Lock; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler; import org.eclipse.jetty.util.thread.Scheduler; @@ -1264,7 +1263,7 @@ public class SessionHandler extends ScopedHandler //1. valid //2. expired //3. idle - try (Lock lock = session.lock()) + try (AutoLock lock = session.lock()) { if (session.getRequests() > 0) return; //session can't expire or be idle if there is a request in it diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java b/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java index 9e82816f2be..1fc1d9523a4 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/IteratingCallback.java @@ -20,7 +20,7 @@ package org.eclipse.jetty.util; import java.io.IOException; -import org.eclipse.jetty.util.thread.Locker; +import org.eclipse.jetty.util.thread.AutoLock; /** * This specialized callback implements a pattern that allows @@ -125,7 +125,7 @@ public abstract class IteratingCallback implements Callback SUCCEEDED } - private Locker _locker = new Locker(); + private final AutoLock _lock = new AutoLock(); private State _state; private boolean _iterate; @@ -188,35 +188,31 @@ public abstract class IteratingCallback implements Callback { boolean process = false; - loop: - while (true) + try (AutoLock lock = _lock.lock()) { - try (Locker.Lock lock = _locker.lock()) + switch (_state) { - switch (_state) - { - case PENDING: - case CALLED: - // process will be called when callback is handled - break loop; + case PENDING: + case CALLED: + // process will be called when callback is handled + break; - case IDLE: - _state = State.PROCESSING; - process = true; - break loop; + case IDLE: + _state = State.PROCESSING; + process = true; + break; - case PROCESSING: - _iterate = true; - break loop; + case PROCESSING: + _iterate = true; + break; - case FAILED: - case SUCCEEDED: - break loop; + case FAILED: + case SUCCEEDED: + break; - case CLOSED: - default: - throw new IllegalStateException(toString()); - } + case CLOSED: + default: + throw new IllegalStateException(toString()); } } if (process) @@ -243,11 +239,11 @@ public abstract class IteratingCallback implements Callback catch (Throwable x) { failed(x); - break processing; + break; } // acted on the action we have just received - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { @@ -295,18 +291,11 @@ public abstract class IteratingCallback implements Callback case CALLED: { - switch (action) - { - case SCHEDULED: - { - // we lost the race, so we have to keep processing - _state = State.PROCESSING; - continue processing; - } - - default: - throw new IllegalStateException(String.format("%s[action=%s]", this, action)); - } + if (action != Action.SCHEDULED) + throw new IllegalStateException(String.format("%s[action=%s]", this, action)); + // we lost the race, so we have to keep processing + _state = State.PROCESSING; + continue processing; } case SUCCEEDED: @@ -335,7 +324,7 @@ public abstract class IteratingCallback implements Callback public void succeeded() { boolean process = false; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { @@ -375,7 +364,7 @@ public abstract class IteratingCallback implements Callback public void failed(Throwable x) { boolean failure = false; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { @@ -405,7 +394,7 @@ public abstract class IteratingCallback implements Callback public void close() { String failure = null; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { @@ -434,7 +423,7 @@ public abstract class IteratingCallback implements Callback */ boolean isIdle() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { return _state == State.IDLE; } @@ -442,7 +431,7 @@ public abstract class IteratingCallback implements Callback public boolean isClosed() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { return _state == State.CLOSED; } @@ -453,7 +442,7 @@ public abstract class IteratingCallback implements Callback */ public boolean isFailed() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { return _state == State.FAILED; } @@ -464,7 +453,7 @@ public abstract class IteratingCallback implements Callback */ public boolean isSucceeded() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { return _state == State.SUCCEEDED; } @@ -481,7 +470,7 @@ public abstract class IteratingCallback implements Callback */ public boolean reset() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/Locker.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/AutoLock.java similarity index 68% rename from jetty-util/src/main/java/org/eclipse/jetty/util/thread/Locker.java rename to jetty-util/src/main/java/org/eclipse/jetty/util/thread/AutoLock.java index b56180bd90e..9bcab7eff2a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/Locker.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/AutoLock.java @@ -22,37 +22,27 @@ import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; /** - *

    Convenience auto closeable {@link java.util.concurrent.locks.ReentrantLock} wrapper.

    - * + * Reentrant lock that can be used in a try-with-resources statement. * 
    - * try (Locker.Lock lock = locker.lock())
+ * try (AutoLock lock = this.lock.lock())
  * {
- *   // something
+ *     // Something
  * }
  *
    */ -public class Locker +public class AutoLock implements AutoCloseable { private final ReentrantLock _lock = new ReentrantLock(); - private final Lock _unlock = new Lock(); /** *

    Acquires the lock.

    * - * @return the lock to unlock + * @return this AutoLock for unlocking */ - public Lock lock() + public AutoLock lock() { _lock.lock(); - return _unlock; - } - - /** - * @return whether this lock has been acquired - */ - public boolean isLocked() - { - return _lock.isLocked(); + return this; } /** @@ -63,15 +53,15 @@ public class Locker return _lock.newCondition(); } - /** - *

    The unlocker object that unlocks when it is closed.

    - */ - public class Lock implements AutoCloseable + // Package-private for testing only. + boolean isLocked() { - @Override - public void close() - { - _lock.unlock(); - } + return _lock.isLocked(); + } + + @Override + public void close() + { + _lock.unlock(); } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java index 3c7fa26e90b..d34792634ef 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/ReservedThreadExecutor.java @@ -248,8 +248,8 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements TryExec private class ReservedThread implements Runnable { - private final Locker _locker = new Locker(); - private final Condition _wakeup = _locker.newCondition(); + private final AutoLock _lock = new AutoLock(); + private final Condition _wakeup = _lock.newCondition(); private boolean _starting = true; private Runnable _task = null; @@ -258,7 +258,7 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements TryExec if (LOG.isDebugEnabled()) LOG.debug("{} offer {}", this, task); - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { _task = task; _wakeup.signal(); @@ -280,7 +280,7 @@ public class ReservedThreadExecutor extends AbstractLifeCycle implements TryExec { boolean idle = false; - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { if (_task == null) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ExecuteProduceConsume.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ExecuteProduceConsume.java index d32a779f543..bb14efbeb76 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ExecuteProduceConsume.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ExecuteProduceConsume.java @@ -22,11 +22,10 @@ import java.util.concurrent.Executor; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +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.Invocable.InvocationType; -import org.eclipse.jetty.util.thread.Locker; -import org.eclipse.jetty.util.thread.Locker.Lock; /** *

    A strategy where the thread that produces will always run the resulting task.

    @@ -45,7 +44,7 @@ public class ExecuteProduceConsume implements ExecutionStrategy, Runnable { private static final Logger LOG = Log.getLogger(ExecuteProduceConsume.class); - private final Locker _locker = new Locker(); + private final AutoLock _lock = new AutoLock(); private final Runnable _runProduce = new RunProduce(); private final Producer _producer; private final Executor _executor; @@ -67,7 +66,7 @@ public class ExecuteProduceConsume implements ExecutionStrategy, Runnable LOG.debug("{} execute", this); boolean produce = false; - try (Lock locked = _locker.lock()) + try (AutoLock lock = _lock.lock()) { // If we are idle and a thread is not producing if (_idle) @@ -98,7 +97,7 @@ public class ExecuteProduceConsume implements ExecutionStrategy, Runnable if (LOG.isDebugEnabled()) LOG.debug("{} spawning", this); boolean dispatch = false; - try (Lock locked = _locker.lock()) + try (AutoLock lock = _lock.lock()) { if (_idle) dispatch = true; @@ -115,7 +114,7 @@ public class ExecuteProduceConsume implements ExecutionStrategy, Runnable if (LOG.isDebugEnabled()) LOG.debug("{} run", this); boolean produce = false; - try (Lock locked = _locker.lock()) + try (AutoLock lock = _lock.lock()) { _pending = false; if (!_idle && !_producing) @@ -145,7 +144,7 @@ public class ExecuteProduceConsume implements ExecutionStrategy, Runnable LOG.debug("{} produced {}", this, task); boolean dispatch = false; - try (Lock locked = _locker.lock()) + try (AutoLock lock = _lock.lock()) { // Finished producing _producing = false; @@ -191,13 +190,12 @@ public class ExecuteProduceConsume implements ExecutionStrategy, Runnable // Run the task. if (LOG.isDebugEnabled()) LOG.debug("{} run {}", this, task); - if (task != null) - task.run(); + task.run(); if (LOG.isDebugEnabled()) LOG.debug("{} ran {}", this, task); // Once we have run the task, we can try producing again. - try (Lock locked = _locker.lock()) + try (AutoLock lock = _lock.lock()) { // Is another thread already producing or we are now idle? if (_producing || _idle) @@ -212,7 +210,7 @@ public class ExecuteProduceConsume implements ExecutionStrategy, Runnable public Boolean isIdle() { - try (Lock locked = _locker.lock()) + try (AutoLock lock = _lock.lock()) { return _idle; } @@ -223,7 +221,7 @@ public class ExecuteProduceConsume implements ExecutionStrategy, Runnable { StringBuilder builder = new StringBuilder(); builder.append("EPC "); - try (Lock locked = _locker.lock()) + try (AutoLock lock = _lock.lock()) { builder.append(_idle ? "Idle/" : ""); builder.append(_producing ? "Prod/" : ""); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ProduceConsume.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ProduceConsume.java index f37a94a40cf..15de66f6694 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ProduceConsume.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ProduceConsume.java @@ -22,8 +22,8 @@ import java.util.concurrent.Executor; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.thread.AutoLock; import org.eclipse.jetty.util.thread.ExecutionStrategy; -import org.eclipse.jetty.util.thread.Locker; /** *

    A strategy where the caller thread iterates over task production, submitting each @@ -33,7 +33,7 @@ public class ProduceConsume implements ExecutionStrategy, Runnable { private static final Logger LOG = Log.getLogger(ExecuteProduceConsume.class); - private final Locker _locker = new Locker(); + private final AutoLock _lock = new AutoLock(); private final Producer _producer; private final Executor _executor; private State _state = State.IDLE; @@ -47,7 +47,7 @@ public class ProduceConsume implements ExecutionStrategy, Runnable @Override public void produce() { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { @@ -73,7 +73,7 @@ public class ProduceConsume implements ExecutionStrategy, Runnable if (task == null) { - try (Locker.Lock lock = _locker.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ProduceExecuteConsume.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ProduceExecuteConsume.java index 0b7dc16c7bf..a648cc4e118 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ProduceExecuteConsume.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/strategy/ProduceExecuteConsume.java @@ -22,11 +22,10 @@ import java.util.concurrent.Executor; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +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.Invocable.InvocationType; -import org.eclipse.jetty.util.thread.Locker; -import org.eclipse.jetty.util.thread.Locker.Lock; /** *

    A strategy where the caller thread iterates over task production, submitting each @@ -36,7 +35,7 @@ public class ProduceExecuteConsume implements ExecutionStrategy { private static final Logger LOG = Log.getLogger(ProduceExecuteConsume.class); - private final Locker _locker = new Locker(); + private final AutoLock _lock = new AutoLock(); private final Producer _producer; private final Executor _executor; private State _state = State.IDLE; @@ -50,7 +49,7 @@ public class ProduceExecuteConsume implements ExecutionStrategy @Override public void produce() { - try (Lock locked = _locker.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { @@ -77,7 +76,7 @@ public class ProduceExecuteConsume implements ExecutionStrategy if (task == null) { - try (Lock locked = _locker.lock()) + try (AutoLock lock = _lock.lock()) { switch (_state) { @@ -106,7 +105,7 @@ public class ProduceExecuteConsume implements ExecutionStrategy @Override public void dispatch() { - _executor.execute(() -> produce()); + _executor.execute(this::produce); } private enum State diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/LockerTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/AutoLockTest.java similarity index 71% rename from jetty-util/src/test/java/org/eclipse/jetty/util/thread/LockerTest.java rename to jetty-util/src/test/java/org/eclipse/jetty/util/thread/AutoLockTest.java index 967969d771d..3b388d0771e 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/thread/LockerTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/thread/AutoLockTest.java @@ -26,19 +26,15 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -public class LockerTest +public class AutoLockTest { - public LockerTest() - { - } - @Test public void testLocked() { - Locker lock = new Locker(); + AutoLock lock = new AutoLock(); assertFalse(lock.isLocked()); - try (Locker.Lock l = lock.lock()) + try (AutoLock l = lock.lock()) { assertTrue(lock.isLocked()); } @@ -53,10 +49,10 @@ public class LockerTest @Test public void testLockedException() { - Locker lock = new Locker(); + AutoLock lock = new AutoLock(); assertFalse(lock.isLocked()); - try (Locker.Lock l = lock.lock()) + try (AutoLock l = lock.lock()) { assertTrue(lock.isLocked()); throw new Exception(); @@ -76,27 +72,23 @@ public class LockerTest @Test public void testContend() throws Exception { - final Locker lock = new Locker(); + AutoLock lock = new AutoLock(); final CountDownLatch held0 = new CountDownLatch(1); final CountDownLatch hold0 = new CountDownLatch(1); - Thread thread0 = new Thread() + Thread thread0 = new Thread(() -> { - @Override - public void run() + try (AutoLock l = lock.lock()) { - try (Locker.Lock l = lock.lock()) - { - held0.countDown(); - hold0.await(); - } - catch (InterruptedException e) - { - e.printStackTrace(); - } + held0.countDown(); + hold0.await(); } - }; + catch (InterruptedException e) + { + e.printStackTrace(); + } + }); thread0.start(); held0.await(); @@ -104,22 +96,18 @@ public class LockerTest final CountDownLatch held1 = new CountDownLatch(1); final CountDownLatch hold1 = new CountDownLatch(1); - Thread thread1 = new Thread() + Thread thread1 = new Thread(() -> { - @Override - public void run() + try (AutoLock l = lock.lock()) { - try (Locker.Lock l = lock.lock()) - { - held1.countDown(); - hold1.await(); - } - catch (InterruptedException e) - { - e.printStackTrace(); - } + held1.countDown(); + hold1.await(); } - }; + catch (InterruptedException e) + { + e.printStackTrace(); + } + }); thread1.start(); // thread1 will be spinning here assertFalse(held1.await(100, TimeUnit.MILLISECONDS)); diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/IdleSessionTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/IdleSessionTest.java index c66713e9f72..1074f0f8cd9 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/IdleSessionTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/IdleSessionTest.java @@ -34,11 +34,13 @@ import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.StacklessLogging; -import org.eclipse.jetty.util.thread.Locker.Lock; +import org.eclipse.jetty.util.thread.AutoLock; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -94,7 +96,7 @@ public class IdleSessionTest ContentResponse response = client.GET(url + "?action=init"); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); String sessionCookie = response.getHeaders().get("Set-Cookie"); - assertTrue(sessionCookie != null); + assertNotNull(sessionCookie); //ensure request has finished being handled synchronizer.await(5, TimeUnit.SECONDS); @@ -148,7 +150,7 @@ public class IdleSessionTest response = client.GET(url + "?action=init"); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); sessionCookie = response.getHeaders().get("Set-Cookie"); - assertTrue(sessionCookie != null); + assertNotNull(sessionCookie); id = TestServer.extractSessionId(sessionCookie); //ensure request has finished being handled @@ -220,7 +222,7 @@ public class IdleSessionTest ContentResponse response = client.GET(url + "?action=init"); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); String sessionCookie = response.getHeaders().get("Set-Cookie"); - assertTrue(sessionCookie != null); + assertNotNull(sessionCookie); //ensure request has finished being handled synchronizer.await(5, TimeUnit.SECONDS); @@ -265,7 +267,7 @@ public class IdleSessionTest response = client.GET(url + "?action=init"); assertEquals(HttpServletResponse.SC_OK, response.getStatus()); sessionCookie = response.getHeaders().get("Set-Cookie"); - assertTrue(sessionCookie != null); + assertNotNull(sessionCookie); id = TestServer.extractSessionId(sessionCookie); //ensure request has finished being handled @@ -317,7 +319,7 @@ public class IdleSessionTest session.setAttribute("value", 1); originalId = session.getId(); Session s = (Session)session; - try (Lock lock = s.lock()) + try (AutoLock lock = s.lock()) { assertTrue(s.isResident()); } @@ -326,21 +328,21 @@ public class IdleSessionTest else if ("test".equals(action)) { HttpSession session = request.getSession(false); - assertTrue(session != null); - assertTrue(originalId.equals(session.getId())); + assertNotNull(session); + assertEquals(originalId, session.getId()); Session s = (Session)session; - try (Lock lock = s.lock();) + try (AutoLock lock = s.lock()) { assertTrue(s.isResident()); } Integer v = (Integer)session.getAttribute("value"); - session.setAttribute("value", v.intValue() + 1); + session.setAttribute("value", v + 1); _session = session; } else if ("testfail".equals(action)) { HttpSession session = request.getSession(false); - assertTrue(session == null); + assertNull(session); _session = session; } }