Merge pull request #6094 from eclipse/jetty-10.0.x-WebSocketServerComponents

Do not store WebSocketComponents in Server scope.

jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java

@@ -71,7 +71,7 @@ public class DeflaterPool extends CompressionPool<Deflater>
             capacity = threadPool.getMaxThreads();
 
         pool = new DeflaterPool(capacity, Deflater.DEFAULT_COMPRESSION, true);
-        container.addBean(pool);
+        container.addBean(pool, true);
         return pool;
     }
 }

jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java

@@ -68,7 +68,7 @@ public class InflaterPool extends CompressionPool<Inflater>
             capacity = threadPool.getMaxThreads();
 
         pool = new InflaterPool(capacity, true);
-        container.addBean(pool);
+        container.addBean(pool, true);
         return pool;
     }
 }

jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java

@@ -51,6 +51,9 @@ public class WebSocketComponents extends ContainerLifeCycle
 
         addBean(inflaterPool);
         addBean(deflaterPool);
+        addBean(bufferPool);
+        addBean(extensionRegistry);
+        addBean(objectFactory);
     }
 
     public ByteBufferPool getBufferPool()

jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java

@@ -14,10 +14,13 @@
 package org.eclipse.jetty.websocket.core.server;
 
 import javax.servlet.ServletContext;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
 
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.util.DecoratedObjectFactory;
+import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.util.compression.DeflaterPool;
 import org.eclipse.jetty.util.compression.InflaterPool;
 import org.eclipse.jetty.websocket.core.WebSocketComponents;
@@ -35,31 +38,70 @@ public class WebSocketServerComponents extends WebSocketComponents
     public static final String WEBSOCKET_COMPONENTS_ATTRIBUTE = WebSocketComponents.class.getName();
     public static final String WEBSOCKET_INFLATER_POOL_ATTRIBUTE = "jetty.websocket.inflater";
     public static final String WEBSOCKET_DEFLATER_POOL_ATTRIBUTE = "jetty.websocket.deflater";
+    public static final String WEBSOCKET_BUFFER_POOL_ATTRIBUTE = "jetty.websocket.bufferPool";
 
-    WebSocketServerComponents(InflaterPool inflaterPool, DeflaterPool deflaterPool)
+    WebSocketServerComponents(InflaterPool inflaterPool, DeflaterPool deflaterPool, ByteBufferPool bufferPool)
     {
-        super(null, null, null, inflaterPool, deflaterPool);
+        super(null, null, bufferPool, inflaterPool, deflaterPool);
     }
 
+    /**
+     * <p>
+     * This ensures a {@link WebSocketComponents} is available at the {@link ServletContext} attribute {@link #WEBSOCKET_COMPONENTS_ATTRIBUTE}.
+     * </p>
+     * <p>
+     * This should be called when the server is starting, usually by a {@link javax.servlet.ServletContainerInitializer}.
+     * </p>
+     * <p>
+     * Servlet context attributes can be set with {@link #WEBSOCKET_BUFFER_POOL_ATTRIBUTE}, {@link #WEBSOCKET_INFLATER_POOL_ATTRIBUTE}
+     * and {@link #WEBSOCKET_DEFLATER_POOL_ATTRIBUTE} to override the {@link ByteBufferPool}, {@link DeflaterPool} or
+     * {@link InflaterPool} used by the components, otherwise this will try to use the pools shared on the {@link Server}.
+     * </p>
+     * @param server the server.
+     * @param servletContext the ServletContext.
+     * @return the WebSocketComponents that was created or found on the ServletContext.
+     */
     public static WebSocketComponents ensureWebSocketComponents(Server server, ServletContext servletContext)
     {
-        WebSocketComponents components = server.getBean(WebSocketComponents.class);
+        WebSocketComponents components = (WebSocketComponents)servletContext.getAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE);
-        if (components == null)
+        if (components != null)
+            return components;
+
+        InflaterPool inflaterPool = (InflaterPool)servletContext.getAttribute(WEBSOCKET_INFLATER_POOL_ATTRIBUTE);
+        if (inflaterPool == null)
+            inflaterPool = InflaterPool.ensurePool(server);
+
+        DeflaterPool deflaterPool = (DeflaterPool)servletContext.getAttribute(WEBSOCKET_DEFLATER_POOL_ATTRIBUTE);
+        if (deflaterPool == null)
+            deflaterPool = DeflaterPool.ensurePool(server);
+
+        ByteBufferPool bufferPool = (ByteBufferPool)servletContext.getAttribute(WEBSOCKET_BUFFER_POOL_ATTRIBUTE);
+        if (bufferPool == null)
+            bufferPool = server.getBean(ByteBufferPool.class);
+
+        WebSocketComponents serverComponents = new WebSocketServerComponents(inflaterPool, deflaterPool, bufferPool);
+
+        // These components may be managed by the server but not yet started.
+        // In this case we don't want them to be managed by the components as well.
+        if (server.contains(inflaterPool))
+            serverComponents.unmanage(inflaterPool);
+        if (server.contains(deflaterPool))
+            serverComponents.unmanage(deflaterPool);
+        if (server.contains(bufferPool))
+            serverComponents.unmanage(bufferPool);
+
+        servletContext.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, serverComponents);
+        LifeCycle.start(serverComponents);
+        servletContext.addListener(new ServletContextListener()
         {
-            InflaterPool inflaterPool = (InflaterPool)servletContext.getAttribute(WEBSOCKET_INFLATER_POOL_ATTRIBUTE);
+            @Override
-            if (inflaterPool == null)
+            public void contextDestroyed(ServletContextEvent sce)
-                inflaterPool = InflaterPool.ensurePool(server);
+            {
+                LifeCycle.stop(serverComponents);
+            }
+        });
 
-            DeflaterPool deflaterPool = (DeflaterPool)servletContext.getAttribute(WEBSOCKET_DEFLATER_POOL_ATTRIBUTE);
+        return serverComponents;
-            if (deflaterPool == null)
-                deflaterPool = DeflaterPool.ensurePool(server);
-
-            components = new WebSocketServerComponents(inflaterPool, deflaterPool);
-            server.addBean(components);
-        }
-
-        servletContext.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, components);
-        return components;
     }
 
     public static WebSocketComponents getWebSocketComponents(ServletContext servletContext)

jetty-websocket/websocket-core-tests/pom.xml

@@ -30,6 +30,12 @@
       <artifactId>websocket-core-common</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>org.eclipse.jetty</groupId>
+      <artifactId>jetty-servlet</artifactId>
+      <version>${project.version}</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.eclipse.jetty</groupId>
       <artifactId>jetty-slf4j-impl</artifactId>

jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java

@@ -0,0 +1,152 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.websocket.core;
+
+import java.util.zip.Deflater;
+
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.handler.ContextHandler;
+import org.eclipse.jetty.servlet.ServletContainerInitializerHolder;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.util.compression.DeflaterPool;
+import org.eclipse.jetty.util.compression.InflaterPool;
+import org.eclipse.jetty.websocket.core.server.WebSocketServerComponents;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class WebSocketServerComponentsTest
+{
+    private Server server;
+    private ServletContextHandler contextHandler;
+    private WebSocketComponents components;
+
+    @BeforeEach
+    public void before()
+    {
+        server = new Server();
+        contextHandler = new ServletContextHandler();
+        server.setHandler(contextHandler);
+    }
+
+    @AfterEach
+    public void after() throws Exception
+    {
+        server.stop();
+    }
+
+    @Test
+    public void testComponentsInsideServletContainerInitializer() throws Exception
+    {
+        // ensureWebSocketComponents can only be called when the server is starting.
+        contextHandler.addServletContainerInitializer(new ServletContainerInitializerHolder((c, ctx) ->
+            components = WebSocketServerComponents.ensureWebSocketComponents(server, contextHandler.getServletContext())));
+
+        // Components is created only when the server is started.
+        assertNull(components);
+        server.start();
+        assertNotNull(components);
+
+        // Components is started when it is created.
+        assertTrue(components.isStarted());
+        DeflaterPool deflaterPool = components.getDeflaterPool();
+        InflaterPool inflaterPool = components.getInflaterPool();
+
+        // The components is stopped with the ServletContext.
+        contextHandler.stop();
+        assertTrue(components.isStopped());
+
+        // By default the default CompressionPools from the server are used, these should not be stopped with the context.
+        assertTrue(inflaterPool.isStarted());
+        assertTrue(deflaterPool.isStarted());
+    }
+
+    @Test
+    public void testCompressionPoolsManagedByContext() throws Exception
+    {
+        ContextHandler.Context servletContext = contextHandler.getServletContext();
+
+        // Use a custom InflaterPool and DeflaterPool that are not started or managed.
+        InflaterPool inflaterPool = new InflaterPool(333, false);
+        DeflaterPool deflaterPool = new DeflaterPool(333, Deflater.BEST_SPEED, false);
+        servletContext.setAttribute(WebSocketServerComponents.WEBSOCKET_DEFLATER_POOL_ATTRIBUTE, deflaterPool);
+        servletContext.setAttribute(WebSocketServerComponents.WEBSOCKET_INFLATER_POOL_ATTRIBUTE, inflaterPool);
+
+        // ensureWebSocketComponents can only be called when the server is starting.
+        contextHandler.addServletContainerInitializer(new ServletContainerInitializerHolder((c, ctx) ->
+            components = WebSocketServerComponents.ensureWebSocketComponents(server, servletContext)));
+
+        // Components is created only when the server is started.
+        assertNull(components);
+        server.start();
+        assertNotNull(components);
+
+        // Components is started when it is created.
+        assertTrue(components.isStarted());
+
+        // The components uses the CompressionPools set as servletContext attributes.
+        assertThat(components.getInflaterPool(), is(inflaterPool));
+        assertThat(components.getDeflaterPool(), is(deflaterPool));
+        assertTrue(inflaterPool.isStarted());
+        assertTrue(deflaterPool.isStarted());
+
+        // The components is stopped with the ServletContext.
+        contextHandler.stop();
+        assertTrue(components.isStopped());
+
+        // The inflater and deflater pools are stopped as they are not managed by the server.
+        assertTrue(inflaterPool.isStopped());
+        assertTrue(deflaterPool.isStopped());
+    }
+
+    @Test
+    public void testCompressionPoolsManagedByServer() throws Exception
+    {
+        ContextHandler.Context servletContext = contextHandler.getServletContext();
+
+        // Use a custom InflaterPool and DeflaterPool that are not started or managed.
+        InflaterPool inflaterPool = new InflaterPool(333, false);
+        DeflaterPool deflaterPool = new DeflaterPool(333, Deflater.BEST_SPEED, false);
+        server.addBean(inflaterPool);
+        server.addBean(deflaterPool);
+
+        // ensureWebSocketComponents can only be called when the server is starting.
+        contextHandler.addServletContainerInitializer(new ServletContainerInitializerHolder((c, ctx) ->
+            components = WebSocketServerComponents.ensureWebSocketComponents(server, servletContext)));
+
+        // The CompressionPools will only be started with the server.
+        assertTrue(inflaterPool.isStopped());
+        assertTrue(deflaterPool.isStopped());
+        server.start();
+        assertThat(components.getInflaterPool(), is(inflaterPool));
+        assertThat(components.getDeflaterPool(), is(deflaterPool));
+        assertTrue(inflaterPool.isStarted());
+        assertTrue(deflaterPool.isStarted());
+
+        // The components is stopped with the ServletContext, but the CompressionPools are stopped with the server.
+        contextHandler.stop();
+        assertTrue(components.isStopped());
+        assertTrue(inflaterPool.isStarted());
+        assertTrue(deflaterPool.isStarted());
+        server.stop();
+        assertTrue(inflaterPool.isStopped());
+        assertTrue(deflaterPool.isStopped());
+    }
+}