Jetty 12 recycle servlet channel

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Components.java

@@ -13,15 +13,13 @@
 
 package org.eclipse.jetty.server;
 
-import java.util.Map;
-
 import org.eclipse.jetty.io.ByteBufferPool;
+import org.eclipse.jetty.util.Attributes;
 import org.eclipse.jetty.util.thread.Scheduler;
 import org.eclipse.jetty.util.thread.ThreadPool;
 
 /**
- * Components made available via a {@link Request}
- * TODO flesh out this idea... maybe better name?
+ * Common components made available via a {@link Request}
  */
 public interface Components
 {
@@ -36,12 +34,13 @@ public interface Components
      * The cache will have a life cycle limited by the connection, i.e. no cache map will live
      * longer that the connection associated with it.  However, a cache may have a shorter life
      * than a connection (e.g. it may be discarded for implementation reasons).  A cache map is
-     * guaranteed to be give to only a single request concurrently, so objects saved there do not
+     * guaranteed to be given to only a single request concurrently (scoped by
+     * {@link org.eclipse.jetty.server.internal.HttpChannelState}), so objects saved there do not
      * need to be made safe from access by simultaneous request.
-     * If the connection is known to be none-persistent then the cache may be a noop cache and discard
-     * all items set on it.
+     * If the connection is known to be none-persistent then the cache may be a noop
+     * cache and discard all items set on it.
+     *
      * @return A Map, which may be an empty map that discards all items.
-     * TODO This should be Attributes like everything else.
      */
-    Map<String, Object> getCache();
+    Attributes getCache();
 }

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java

@@ -117,6 +117,8 @@
 public interface Request extends Attributes, Content.Source
 {
     List<Locale> __defaultLocale = Collections.singletonList(Locale.getDefault());
+    String CACHE_ATTRIBUTE = Request.class.getCanonicalName() + ".CookieCache";
+    String COOKIE_ATTRIBUTE = Request.class.getCanonicalName() + ".Cookies";
 
     /**
      * an ID unique within the lifetime scope of the {@link ConnectionMetaData#getId()}).
@@ -388,21 +390,21 @@ static Fields extractQueryParameters(Request request, Charset charset)
     static List<HttpCookie> getCookies(Request request)
     {
         // TODO modify Request and HttpChannel to be optimised for the known attributes
-        List<HttpCookie> cookies = (List<HttpCookie>)request.getAttribute(Request.class.getCanonicalName() + ".Cookies");
+        List<HttpCookie> cookies = (List<HttpCookie>)request.getAttribute(COOKIE_ATTRIBUTE);
         if (cookies != null)
             return cookies;
 
         // TODO: review whether to store the cookie cache at the connection level, or whether to cache them at all.
-        CookieCache cookieCache = (CookieCache)request.getComponents().getCache().get(Request.class.getCanonicalName() + ".CookieCache");
+        CookieCache cookieCache = (CookieCache)request.getComponents().getCache().getAttribute(CACHE_ATTRIBUTE);
         if (cookieCache == null)
         {
             // TODO compliance listeners?
             cookieCache = new CookieCache(request.getConnectionMetaData().getHttpConfiguration().getRequestCookieCompliance(), null);
-            request.getComponents().getCache().put(Request.class.getCanonicalName() + ".CookieCache", cookieCache);
+            request.getComponents().getCache().setAttribute(CACHE_ATTRIBUTE, cookieCache);
         }
 
         cookies = cookieCache.getCookies(request.getHeaders());
-        request.setAttribute(Request.class.getCanonicalName() + ".Cookies", cookies);
+        request.setAttribute(COOKIE_ATTRIBUTE, cookies);
         return cookies;
     }
 

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java

@@ -169,7 +169,7 @@ enum WriteState
     private Callback _writeCallback;
     private Content.Chunk.Error _error;
     private Predicate<Throwable> _onError;
-    private Map<String, Object> _cache;
+    private Attributes _cache;
 
     public HttpChannelState(ConnectionMetaData connectionMetaData)
     {
@@ -321,14 +321,14 @@ public ThreadPool getThreadPool()
     }
 
     @Override
-    public Map<String, Object> getCache()
+    public Attributes getCache()
     {
         if (_cache == null)
         {
             if (getConnectionMetaData().isPersistent())
-                _cache = new HashMap<>();
+                _cache = new Attributes.Mapped(new HashMap<>());
             else
-                _cache = NULL_CACHE;
+                _cache = Attributes.NULL;
         }
         return _cache;
     }

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Attributes.java

@@ -125,7 +125,6 @@ public int size()
         };
     }
 
-
     /**
      * Clear all attribute names
      */
@@ -225,22 +224,31 @@ public boolean equals(Object obj)
         {
             return getWrapped().equals(obj);
         }
+
     }
 
     /**
      * An Attributes implementation backed by a {@link ConcurrentHashMap}.
      */
     class Mapped implements Attributes
     {
-        private final java.util.concurrent.ConcurrentMap<String, Object> _map = new ConcurrentHashMap<>();
-        private final Set<String> _names = Collections.unmodifiableSet(_map.keySet());
+        private final Map<String, Object> _map;
+        private final Set<String> _names;
 
         public Mapped()
         {
+            this(new ConcurrentHashMap<>());
+        }
+
+        public Mapped(Map<String, Object> map)
+        {
+            _map = Objects.requireNonNull(map);
+            _names = Collections.unmodifiableSet(_map.keySet());
         }
 
         public Mapped(Mapped attributes)
         {
+            this();
             _map.putAll(attributes._map);
         }
 
@@ -567,4 +575,42 @@ public boolean equals(Object o)
             return false;
         }
     }
+
+    Attributes NULL = new Attributes()
+    {
+        @Override
+        public Object removeAttribute(String name)
+        {
+            return null;
+        }
+
+        @Override
+        public Object setAttribute(String name, Object attribute)
+        {
+            return null;
+        }
+
+        @Override
+        public Object getAttribute(String name)
+        {
+            return null;
+        }
+
+        @Override
+        public Set<String> getAttributeNameSet()
+        {
+            return Collections.emptySet();
+        }
+
+        @Override
+        public void clearAttributes()
+        {
+        }
+
+        @Override
+        public Map<String, Object> asAttributeMap()
+        {
+            return Collections.emptyMap();
+        }
+    };
 }

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContentProducer.java

@@ -38,7 +38,7 @@ class AsyncContentProducer implements ContentProducer
     private static final Logger LOG = LoggerFactory.getLogger(AsyncContentProducer.class);
     private static final Content.Chunk.Error RECYCLED_ERROR_CHUNK = Content.Chunk.from(new StaticException("ContentProducer has been recycled"));
 
-    private final AutoLock _lock = new AutoLock();
+    final AutoLock _lock;
     private final ServletChannel _servletChannel;
     private HttpInput.Interceptor _interceptor;
     private Content.Chunk _rawChunk;
@@ -47,15 +47,14 @@ class AsyncContentProducer implements ContentProducer
     private long _firstByteNanoTime = Long.MIN_VALUE;
     private long _rawBytesArrived;
 
-    AsyncContentProducer(ServletChannel servletChannel)
+    /**
+     * @param servletChannel The ServletChannel to produce input from.
+     * @param lock The lock of the HttpInput, shared with this instance
+     */
+    AsyncContentProducer(ServletChannel servletChannel, AutoLock lock)
     {
         _servletChannel = servletChannel;
-    }
-
-    @Override
-    public AutoLock lock()
-    {
-        return _lock.lock();
+        _lock = lock;
     }
 
     @Override
@@ -225,7 +224,7 @@ private boolean consumeCurrentChunk()
 
     private boolean consumeAvailableChunks()
     {
-        ServletContextRequest request = _servletChannel.getRequest();
+        ServletContextRequest request = _servletChannel.getServletContextRequest();
         while (true)
         {
             Content.Chunk chunk = request.read();
@@ -288,7 +287,7 @@ public boolean isReady()
         }
 
         _servletChannel.getState().onReadUnready();
-        _servletChannel.getRequest().demand(() ->
+        _servletChannel.getServletContextRequest().demand(() ->
         {
             if (_servletChannel.getHttpInput().onContentProducible())
                 _servletChannel.handle();
@@ -481,7 +480,7 @@ else if (chunk != _rawChunk && !_rawChunk.isTerminal() && _rawChunk.hasRemaining
 
     private Content.Chunk produceRawChunk()
     {
-        Content.Chunk chunk = _servletChannel.getRequest().read();
+        Content.Chunk chunk = _servletChannel.getServletContextRequest().read();
         if (chunk != null)
         {
             _rawBytesArrived += chunk.remaining();
@@ -523,7 +522,7 @@ LockedSemaphore newLockedSemaphore()
     }
 
     /**
-     * A semaphore that assumes working under {@link AsyncContentProducer#lock()} scope.
+     * A semaphore that assumes working under the same locked scope.
      */
     class LockedSemaphore
     {

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/AsyncContextState.java

@@ -116,7 +116,7 @@ public boolean hasOriginalRequestAndResponse()
         ServletRequest servletRequest = getRequest();
         ServletResponse servletResponse = getResponse();
         ServletChannel servletChannel = _state.getServletChannel();
-        HttpServletRequest originalHttpServletRequest = servletChannel.getRequest().getHttpServletRequest();
+        HttpServletRequest originalHttpServletRequest = servletChannel.getServletContextRequest().getHttpServletRequest();
         HttpServletResponse originalHttpServletResponse = servletChannel.getResponse().getHttpServletResponse();
         return (servletRequest == originalHttpServletRequest && servletResponse == originalHttpServletResponse);
     }

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/BlockingContentProducer.java

@@ -14,7 +14,6 @@
 package org.eclipse.jetty.ee10.servlet;
 
 import org.eclipse.jetty.io.Content;
-import org.eclipse.jetty.util.thread.AutoLock;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -29,18 +28,15 @@ class BlockingContentProducer implements ContentProducer
     private final AsyncContentProducer _asyncContentProducer;
     private final AsyncContentProducer.LockedSemaphore _semaphore;
 
-    BlockingContentProducer(AsyncContentProducer delegate)
+    /**
+     * @param asyncContentProducer The {@link AsyncContentProducer} to block against.
+     */
+    BlockingContentProducer(AsyncContentProducer asyncContentProducer)
     {
-        _asyncContentProducer = delegate;
+        _asyncContentProducer = asyncContentProducer;
         _semaphore = _asyncContentProducer.newLockedSemaphore();
     }
 
-    @Override
-    public AutoLock lock()
-    {
-        return _asyncContentProducer.lock();
-    }
-
     @Override
     public void recycle()
     {

jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ContentProducer.java

@@ -15,20 +15,12 @@
 
 import org.eclipse.jetty.io.Content;
 import org.eclipse.jetty.util.component.Destroyable;
-import org.eclipse.jetty.util.thread.AutoLock;
 
 /**
  * ContentProducer is the bridge between {@link HttpInput} and {@link Content.Source}.
  */
 public interface ContentProducer
 {
-    /**
-     * Lock this instance. The lock must be held before any of this instance's
-     * method can be called, and must be released afterward.
-     * @return the lock that is guarding this instance.
-     */
-    AutoLock lock();
-
     /**
      * Clear the interceptor and call {@link Destroyable#destroy()} on it if it implements {@link Destroyable}.
      * A recycled {@link ContentProducer} will only produce special content with a non-null error until