Issue #6974 - WebSocket should user server ByteBufferPool if possible

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · Oct 19, 2021 · 53ff86f · 53ff86f
1 parent a0f4bb9
commit 53ff86f
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 10 deletions.
diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/ArrayByteBufferPoolTest.java
@@ -28,6 +28,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.lessThan;
 import static org.hamcrest.Matchers.lessThanOrEqualTo;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -69,10 +70,13 @@ public void testMinimumRelease()
     @Test
     public void testMaxRelease()
     {
-        ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(10, 100, 1000);
+        int minCapacity = 10;
+        int factor = 1;
+        int maxCapacity = 1024;
+        ArrayByteBufferPool bufferPool = new ArrayByteBufferPool(minCapacity, factor, maxCapacity);
         ByteBufferPool.Bucket[] buckets = bufferPool.bucketsFor(true);
 
-        for (int size = 999; size <= 1001; size++)
+        for (int size = maxCapacity - 1; size <= maxCapacity + 1; size++)
         {
             bufferPool.clear();
             ByteBuffer buffer = bufferPool.acquire(size, true);
@@ -91,7 +95,11 @@ public void testMaxRelease()
                 .filter(Objects::nonNull)
                 .mapToInt(Bucket::size)
                 .sum();
-            assertEquals(size <= 1000, 1 == pooled);
+
+            if (size <= maxCapacity)
+                assertThat(pooled, is(1));
+            else
+                assertThat(pooled, is(0));
         }
     }
 

diff --git a/...ocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java b/...ocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java
@@ -46,7 +46,9 @@
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.server.HttpConnection;
 import org.eclipse.jetty.server.HttpConnectionFactory;
+import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.handler.ContextHandler;
+import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.util.DecoratedObjectFactory;
 import org.eclipse.jetty.util.DeprecationWarning;
 import org.eclipse.jetty.util.StringUtil;
@@ -114,7 +116,7 @@ public WebSocketServerFactory()
 
     public WebSocketServerFactory(ServletContext context)
     {
-        this(context, WebSocketPolicy.newServerPolicy(), new MappedByteBufferPool());
+        this(context, WebSocketPolicy.newServerPolicy(), null);
     }
 
     public WebSocketServerFactory(ServletContext context, ByteBufferPool bufferPool)
@@ -130,7 +132,7 @@ public WebSocketServerFactory(ServletContext context, ByteBufferPool bufferPool)
      */
     public WebSocketServerFactory(ServletContext context, WebSocketPolicy policy)
     {
-        this(context, policy, new MappedByteBufferPool());
+        this(context, policy, null);
     }
 
     public WebSocketServerFactory(ServletContext context, WebSocketPolicy policy, ByteBufferPool bufferPool)
@@ -156,7 +158,22 @@ private WebSocketServerFactory(ServletContext context, WebSocketPolicy policy, D
         this.defaultPolicy = policy;
         this.objectFactory = objectFactory;
         this.executor = executor;
+
+        if (bufferPool == null)
+        {
+            ContextHandler contextHandler = ServletContextHandler.getContextHandler(context);
+            if (contextHandler != null)
+            {
+                Server server = contextHandler.getServer();
+                if (server != null)
+                    bufferPool = server.getBean(ByteBufferPool.class);
+            }
+            if (bufferPool == null)
+                bufferPool = new MappedByteBufferPool();
+        }
         this.bufferPool = bufferPool;
+        addBean(bufferPool);
+
 
         this.creator = this;
         this.contextClassloader = Thread.currentThread().getContextClassLoader();
@@ -185,7 +202,6 @@ private WebSocketServerFactory(ServletContext context, WebSocketPolicy policy, D
         supportedVersions = rv.toString();
 
         addBean(scheduler);
-        addBean(bufferPool);
         addBean(sessionTracker);
         addBean(extensionFactory);
         listeners.add(this.sessionTracker);

diff --git a/...ocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java b/...ocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketUpgradeFilter.java
@@ -77,8 +77,7 @@ public static WebSocketUpgradeFilter configure(ServletContextHandler context) th
         if (filter == null)
         {
             // Dynamically add filter
-            NativeWebSocketConfiguration configuration = NativeWebSocketServletContainerInitializer.initialize(context);
-            filter = new WebSocketUpgradeFilter(configuration);
+            filter = new WebSocketUpgradeFilter();
             filter.setToAttribute(context, ATTR_KEY);
 
             String name = "Jetty_WebSocketUpgradeFilter";
@@ -130,7 +129,6 @@ public static WebSocketUpgradeFilter configureContext(ServletContext context) th
     }
 
     private NativeWebSocketConfiguration configuration;
-    private String instanceKey;
     private boolean localConfiguration = false;
     private boolean alreadySetToAttribute = false;
 
@@ -139,11 +137,13 @@ public WebSocketUpgradeFilter()
         // do nothing
     }
 
+    @Deprecated
     public WebSocketUpgradeFilter(WebSocketServerFactory factory)
     {
         this(new NativeWebSocketConfiguration(factory));
     }
 
+    @Deprecated
     public WebSocketUpgradeFilter(NativeWebSocketConfiguration configuration)
     {
         this.configuration = configuration;
@@ -378,7 +378,7 @@ public void init(FilterConfig config) throws ServletException
                 getFactory().getPolicy().setInputBufferSize(Integer.parseInt(max));
             }
 
-            instanceKey = config.getInitParameter(CONTEXT_ATTRIBUTE_KEY);
+            String instanceKey = config.getInitParameter(CONTEXT_ATTRIBUTE_KEY);
             if (instanceKey == null)
             {
                 // assume default