From b0a953a9fc361be3b853615977a8199b38205133 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 19 Jan 2021 16:30:46 +1100 Subject: [PATCH 1/3] Do not store WebSocketComponents in Server scope. Signed-off-by: Lachlan Roberts --- .../websocket/core/WebSocketComponents.java | 1 + .../server/WebSocketServerComponents.java | 31 ++++++++++--------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java index 34148f2523a..970487bfca5 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java @@ -51,6 +51,7 @@ public class WebSocketComponents extends ContainerLifeCycle addBean(inflaterPool); addBean(deflaterPool); + addBean(bufferPool); } public ByteBufferPool getBufferPool() diff --git a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java index 4e0642a22eb..798fdfcd3af 100644 --- a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java +++ b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java @@ -35,29 +35,32 @@ 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); } public static WebSocketComponents ensureWebSocketComponents(Server server, ServletContext servletContext) { - WebSocketComponents components = server.getBean(WebSocketComponents.class); - if (components == null) - { - InflaterPool inflaterPool = (InflaterPool)servletContext.getAttribute(WEBSOCKET_INFLATER_POOL_ATTRIBUTE); - if (inflaterPool == null) - inflaterPool = InflaterPool.ensurePool(server); + WebSocketComponents components = (WebSocketComponents)servletContext.getAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE); + if (components != null) + return components; - DeflaterPool deflaterPool = (DeflaterPool)servletContext.getAttribute(WEBSOCKET_DEFLATER_POOL_ATTRIBUTE); - if (deflaterPool == null) - deflaterPool = DeflaterPool.ensurePool(server); + InflaterPool inflaterPool = (InflaterPool)servletContext.getAttribute(WEBSOCKET_INFLATER_POOL_ATTRIBUTE); + if (inflaterPool == null) + inflaterPool = InflaterPool.ensurePool(server); - components = new WebSocketServerComponents(inflaterPool, deflaterPool); - server.addBean(components); - } + 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); + + components = new WebSocketServerComponents(inflaterPool, deflaterPool, bufferPool); servletContext.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, components); return components; } From b51e40892d301a6450a91ee02fcb5e2395d535d3 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 26 Mar 2021 00:07:57 +1100 Subject: [PATCH 2/3] Fix lifecycle issues with WebSocketComponents and improve testing. Signed-off-by: Lachlan Roberts --- .../jetty/util/compression/DeflaterPool.java | 2 +- .../jetty/util/compression/InflaterPool.java | 2 +- .../websocket/core/WebSocketComponents.java | 2 + .../server/WebSocketServerComponents.java | 45 +++++- jetty-websocket/websocket-core-tests/pom.xml | 6 + .../core/WebSocketServerComponentsTest.java | 152 ++++++++++++++++++ 6 files changed, 204 insertions(+), 5 deletions(-) create mode 100644 jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java index 72f47cae67b..4b6e480293a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/DeflaterPool.java @@ -71,7 +71,7 @@ public class DeflaterPool extends CompressionPool capacity = threadPool.getMaxThreads(); pool = new DeflaterPool(capacity, Deflater.DEFAULT_COMPRESSION, true); - container.addBean(pool); + container.addBean(pool, true); return pool; } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java index 8f167dab5d9..c7e918b666b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/compression/InflaterPool.java @@ -68,7 +68,7 @@ public class InflaterPool extends CompressionPool capacity = threadPool.getMaxThreads(); pool = new InflaterPool(capacity, true); - container.addBean(pool); + container.addBean(pool, true); return pool; } } diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java index 970487bfca5..f9c6e86e742 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/WebSocketComponents.java @@ -52,6 +52,8 @@ public class WebSocketComponents extends ContainerLifeCycle addBean(inflaterPool); addBean(deflaterPool); addBean(bufferPool); + addBean(extensionRegistry); + addBean(objectFactory); } public ByteBufferPool getBufferPool() diff --git a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java index 798fdfcd3af..bfd41f72cb2 100644 --- a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java +++ b/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; @@ -42,6 +45,22 @@ public class WebSocketServerComponents extends WebSocketComponents super(null, null, bufferPool, inflaterPool, deflaterPool); } + /** + *

+ * This ensures a {@link WebSocketComponents} is available at the {@link ServletContext} attribute {@link #WEBSOCKET_COMPONENTS_ATTRIBUTE}. + *

+ *

+ * This should be called when the server is starting, usually by a {@link javax.servlet.ServletContainerInitializer}. + *

+ *

+ * 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}. + *

+ * @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 = (WebSocketComponents)servletContext.getAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE); @@ -60,9 +79,29 @@ public class WebSocketServerComponents extends WebSocketComponents if (bufferPool == null) bufferPool = server.getBean(ByteBufferPool.class); - components = new WebSocketServerComponents(inflaterPool, deflaterPool, bufferPool); - servletContext.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, components); - return components; + 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.isManaged(inflaterPool)) + serverComponents.unmanage(inflaterPool); + if (server.isManaged(deflaterPool)) + serverComponents.unmanage(deflaterPool); + if (server.isManaged(bufferPool)) + serverComponents.unmanage(bufferPool); + + servletContext.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, serverComponents); + LifeCycle.start(serverComponents); + servletContext.addListener(new ServletContextListener() + { + @Override + public void contextDestroyed(ServletContextEvent sce) + { + LifeCycle.stop(serverComponents); + } + }); + + return serverComponents; } public static WebSocketComponents getWebSocketComponents(ServletContext servletContext) diff --git a/jetty-websocket/websocket-core-tests/pom.xml b/jetty-websocket/websocket-core-tests/pom.xml index ac5387af85f..2ed96136651 100644 --- a/jetty-websocket/websocket-core-tests/pom.xml +++ b/jetty-websocket/websocket-core-tests/pom.xml @@ -30,6 +30,12 @@ websocket-core-common ${project.version} + + org.eclipse.jetty + jetty-servlet + ${project.version} + test + org.eclipse.jetty jetty-slf4j-impl diff --git a/jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java b/jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java new file mode 100644 index 00000000000..a173c3104fd --- /dev/null +++ b/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.addManaged(inflaterPool); + server.addManaged(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()); + } +} From 16b014c2fb2e05e8e732e0dc0496b5975a4cb828 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 30 Mar 2021 17:50:55 +1100 Subject: [PATCH 3/3] Unmanage beans on WSSComponents if they are also beans on server. Signed-off-by: Lachlan Roberts --- .../websocket/core/server/WebSocketServerComponents.java | 6 +++--- .../jetty/websocket/core/WebSocketServerComponentsTest.java | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java index bfd41f72cb2..c8717bacb16 100644 --- a/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java +++ b/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java @@ -83,11 +83,11 @@ public class WebSocketServerComponents extends WebSocketComponents // 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.isManaged(inflaterPool)) + if (server.contains(inflaterPool)) serverComponents.unmanage(inflaterPool); - if (server.isManaged(deflaterPool)) + if (server.contains(deflaterPool)) serverComponents.unmanage(deflaterPool); - if (server.isManaged(bufferPool)) + if (server.contains(bufferPool)) serverComponents.unmanage(bufferPool); servletContext.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, serverComponents); diff --git a/jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java b/jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java index a173c3104fd..bc537e25068 100644 --- a/jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java +++ b/jetty-websocket/websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/WebSocketServerComponentsTest.java @@ -124,8 +124,8 @@ public class WebSocketServerComponentsTest // 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.addManaged(inflaterPool); - server.addManaged(deflaterPool); + server.addBean(inflaterPool); + server.addBean(deflaterPool); // ensureWebSocketComponents can only be called when the server is starting. contextHandler.addServletContainerInitializer(new ServletContainerInitializerHolder((c, ctx) ->