Merge pull request #5084 from eclipse/jetty-10.0.x-convert_synchroniz…

…ed_to_autolock Fixes #5083 - Convert synchronized usages to AutoLock.
jetty · Aug 4, 2020 · a79c0cf · a79c0cf
2 parents dbeac5e + c490355
commit a79c0cf
Show file tree

Hide file tree

Showing 90 changed files with 1,740 additions and 1,397 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 l = _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 l = _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 l = _lock.lock())
         {
             HttpExchange existing = _exchange;
             _exchange = null;
@@ -103,7 +105,7 @@ public boolean disassociate(HttpExchange exchange)
 
     public HttpExchange getHttpExchange()
     {
-        synchronized (this)
+        try (AutoLock l = _lock.lock())
         {
             return _exchange;
         }

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
@@ -37,13 +37,15 @@
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.util.Attachable;
 import org.eclipse.jetty.util.HttpCookieStore;
+import org.eclipse.jetty.util.thread.AutoLock;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public abstract class HttpConnection implements IConnection, Attachable
 {
     private static final Logger LOG = LoggerFactory.getLogger(HttpConnection.class);
 
+    private final AutoLock lock = new AutoLock();
     private final HttpDestination destination;
     private Object attachment;
     private int idleTimeoutGuard;
@@ -89,7 +91,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 l = lock.lock())
         {
             send = idleTimeoutGuard >= 0;
             if (send)
@@ -113,7 +115,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 l = lock.lock())
             {
                 --idleTimeoutGuard;
                 idleTimeoutStamp = System.nanoTime();
@@ -252,7 +254,7 @@ private void applyRequestAuthentication(Request request)
 
     public boolean onIdleTimeout(long idleTimeout)
     {
-        synchronized (this)
+        try (AutoLock l = 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,7 +70,7 @@ public HttpRequest getRequest()
 
     public Throwable getRequestFailure()
     {
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             return requestFailure;
         }
@@ -86,7 +88,7 @@ public HttpResponse getResponse()
 
     public Throwable getResponseFailure()
     {
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             return responseFailure;
         }
@@ -103,7 +105,7 @@ boolean associate(HttpChannel channel)
     {
         boolean result = false;
         boolean abort = false;
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             // Only associate if the exchange state is initial,
             // as the exchange could be already failed.
@@ -127,7 +129,7 @@ boolean associate(HttpChannel channel)
     void disassociate(HttpChannel channel)
     {
         boolean abort = false;
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             if (_channel != channel || requestState != State.TERMINATED || responseState != State.TERMINATED)
                 abort = true;
@@ -140,15 +142,15 @@ void disassociate(HttpChannel channel)
 
     private HttpChannel getHttpChannel()
     {
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             return _channel;
         }
     }
 
     public boolean requestComplete(Throwable failure)
     {
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             return completeRequest(failure);
         }
@@ -167,7 +169,7 @@ private boolean completeRequest(Throwable failure)
 
     public boolean responseComplete(Throwable failure)
     {
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             return completeResponse(failure);
         }
@@ -187,7 +189,7 @@ private boolean completeResponse(Throwable failure)
     public Result terminateRequest()
     {
         Result result = null;
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             if (requestState == State.COMPLETED)
                 requestState = State.TERMINATED;
@@ -204,7 +206,7 @@ public Result terminateRequest()
     public Result terminateResponse()
     {
         Result result = null;
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             if (responseState == State.COMPLETED)
                 responseState = State.TERMINATED;
@@ -224,7 +226,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 l = lock.lock())
         {
             abortRequest = completeRequest(failure);
             abortResponse = completeResponse(failure);
@@ -283,7 +285,7 @@ private void notifyFailureComplete(Throwable failure)
 
     public void resetResponse()
     {
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             responseState = State.PENDING;
             responseFailure = null;
@@ -300,7 +302,7 @@ public void proceed(Throwable failure)
     @Override
     public String toString()
     {
-        synchronized (this)
+        try (AutoLock l = 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 l = lock.lock())
         {
             demand = MathUtils.cappedAdd(demand, n);
             if (stalled)
@@ -126,15 +128,15 @@ protected long demand()
 
     private long demand(LongUnaryOperator operator)
     {
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             return demand = operator.applyAsLong(demand);
         }
     }
 
     protected boolean hasDemandOrStall()
     {
-        synchronized (this)
+        try (AutoLock l = lock.lock())
         {
             stalled = demand <= 0;
             return !stalled;

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
@@ -118,7 +118,7 @@ public void demand()
     public void fail(Throwable failure)
     {
         List<Callback> toFail = List.of();
-        try (AutoLock ignored = lock.lock())
+        try (AutoLock l = lock.lock())
         {
             if (this.failure == null)
             {
@@ -293,15 +293,15 @@ private void notifyFailure(Consumer consumer, Throwable failure)
 
     private void notifyFlush()
     {
-        try (AutoLock ignored = lock.lock())
+        try (AutoLock l = lock.lock())
         {
             flush.signal();
         }
     }
 
     public void flush() throws IOException
     {
-        try (AutoLock ignored = lock.lock())
+        try (AutoLock l = lock.lock())
         {
             try
             {
@@ -327,7 +327,7 @@ public void flush() throws IOException
     public void close()
     {
         boolean produce = false;
-        try (AutoLock ignored = lock.lock())
+        try (AutoLock l = lock.lock())
         {
             if (closed)
                 return;

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
@@ -47,6 +47,7 @@
 import org.eclipse.jetty.util.annotation.Name;
 import org.eclipse.jetty.util.component.ContainerLifeCycle;
 import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.util.thread.AutoLock;
 import org.eclipse.jetty.xml.XmlConfiguration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -125,6 +126,7 @@ void setLifeCycleNode(Node node)
         }
     }
 
+    private final AutoLock _lock = new AutoLock();
     private final List<AppProvider> _providers = new ArrayList<AppProvider>();
     private final AppLifeCycle _lifecycle = new AppLifeCycle();
     private final Queue<AppEntry> _apps = new ConcurrentLinkedQueue<AppEntry>();
@@ -562,13 +564,14 @@ public void requestAppGoal(@Name("appId") String appId, @Name("nodeName") String
         requestAppGoal(appentry, nodeName);
     }
 
-    private synchronized void addOnStartupError(Throwable cause)
+    private void addOnStartupError(Throwable cause)
     {
-        if (onStartupErrors == null)
+        try (AutoLock l = _lock.lock())
         {
-            onStartupErrors = new MultiException();
+            if (onStartupErrors == null)
+                onStartupErrors = new MultiException();
+            onStartupErrors.add(cause);
         }
-        onStartupErrors.add(cause);
     }
 
     /**

diff --git a/.../fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java b/.../fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java
@@ -55,13 +55,15 @@
 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;
 
 public class HttpConnectionOverFCGI extends AbstractConnection implements IConnection, Attachable
 {
     private static final Logger LOG = LoggerFactory.getLogger(HttpConnectionOverFCGI.class);
 
+    private final AutoLock lock = new AutoLock();
     private final LinkedList<Integer> requests = new LinkedList<>();
     private final Map<Integer, HttpChannelOverFCGI> activeChannels = new ConcurrentHashMap<>();
     private final Queue<HttpChannelOverFCGI> idleChannels = new ConcurrentLinkedQueue<>();
@@ -321,7 +323,7 @@ private void failAndClose(Throwable failure)
 
     private int acquireRequest()
     {
-        synchronized (requests)
+        try (AutoLock l = lock.lock())
         {
             int last = requests.getLast();
             int request = last + 1;
@@ -332,7 +334,7 @@ private int acquireRequest()
 
     private void releaseRequest(int request)
     {
-        synchronized (requests)
+        try (AutoLock l = lock.lock())
         {
             requests.removeFirstOccurrence(request);
         }