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 <[email protected]>
jetty · Jul 27, 2020 · 8d69fc4 · 8d69fc4
1 parent a33b0e2
commit 8d69fc4
Show file tree

Hide file tree

Showing 99 changed files with 2,014 additions and 2,116 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
@@ -28,6 +28,7 @@
 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 @@
 public class AnnotationIntrospector
 {
     private static final Logger LOG = LoggerFactory.getLogger(AnnotationIntrospector.class);
+
+    private final AutoLock _lock = new AutoLock();
     private final Set<Class<?>> _introspectedClasses = new HashSet<>();
     private final List<IntrospectableAnnotationHandler> _handlers = new ArrayList<IntrospectableAnnotationHandler>();
     private final WebAppContext _context;
@@ -195,14 +198,13 @@ public void introspect(Object o, Object metaInfo)
 
         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
@@ -19,13 +19,15 @@
 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;
 
 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 boolean associate(HttpExchange exchange)
     {
         boolean result = false;
         boolean abort = true;
-        synchronized (this)
+        try (AutoLock ignored = _lock.lock())
         {
             if (_exchange == null)
             {
@@ -85,7 +87,7 @@ public boolean associate(HttpExchange exchange)
     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 boolean disassociate(HttpExchange exchange)
 
     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
@@ -36,13 +36,15 @@
 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;
 
 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 @@ protected SendFailure send(HttpChannel channel, HttpExchange exchange)
         // 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 @@ protected SendFailure send(HttpChannel channel, HttpExchange exchange)
                 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 @@ private void applyRequestAuthentication(Request request)
 
     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
@@ -23,13 +23,15 @@
 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;
 
 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<Response.ResponseListener> listeners;
@@ -68,10 +70,7 @@ public HttpRequest getRequest()
 
     public Throwable getRequestFailure()
     {
-        synchronized (this)
-        {
-            return requestFailure;
-        }
+        return lock.runLocked(() -> requestFailure);
     }
 
     public List<Response.ResponseListener> getResponseListeners()
@@ -86,10 +85,7 @@ public HttpResponse getResponse()
 
     public Throwable getResponseFailure()
     {
-        synchronized (this)
-        {
-            return responseFailure;
-        }
+        return lock.runLocked(() -> responseFailure);
     }
 
     /**
@@ -103,7 +99,7 @@ boolean associate(HttpChannel channel)
     {
         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 @@ boolean associate(HttpChannel channel)
     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 @@ void disassociate(HttpChannel channel)
 
     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 @@ private boolean completeRequest(Throwable failure)
 
     public boolean responseComplete(Throwable failure)
     {
-        synchronized (this)
-        {
-            return completeResponse(failure);
-        }
+        return lock.runLocked(() -> completeResponse(failure));
     }
 
     private boolean completeResponse(Throwable failure)
@@ -187,7 +174,7 @@ private boolean completeResponse(Throwable failure)
     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 Result terminateRequest()
     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 boolean abort(Throwable failure)
         // 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 @@ private void notifyFailureComplete(Throwable failure)
 
     public void resetResponse()
     {
-        synchronized (this)
+        try (AutoLock ignored = lock.lock())
         {
             responseState = State.PENDING;
             responseFailure = null;
@@ -300,7 +287,7 @@ public void proceed(Throwable failure)
     @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
@@ -42,6 +42,7 @@
 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> responseState = new AtomicReference<>(ResponseState.IDLE);
     private final HttpChannel channel;
     private ContentListeners contentListeners;
@@ -98,7 +100,7 @@ void demand(long n)
             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 @@ protected long demand()
 
     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;