From 938b5b92b8c5a2b55f3a059deb75fa76fbb979ba Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 10 Oct 2025 11:53:48 +1100 Subject: [PATCH 1/2] Review the lifecycle of WebSocket beans Signed-off-by: Lachlan Roberts --- .../jetty/docs/programming/ComponentDocs.java | 2 +- .../org/eclipse/jetty/client/HttpClient.java | 2 +- .../jetty/client/HttpClientTLSTest.java | 5 +- .../quic/quiche/client/QuicheTransport.java | 3 +- .../server/QuicheServerConnectionFactory.java | 2 +- .../jetty/server/handler/ContextHandler.java | 2 +- .../jetty/server/handler/HotSwapHandler.java | 2 +- .../util/component/ContainerLifeCycle.java | 22 ++++- .../core/server/WebSocketMappings.java | 23 +++--- .../server/WebSocketServerComponents.java | 46 ++++------- .../websocket/client/WebSocketClient.java | 2 +- .../server/ServerWebSocketContainer.java | 2 +- .../ee10/servlet/ServletContextHandler.java | 5 ++ .../test/support/XmlBasedJettyServer.java | 3 +- .../JakartaWebSocketClientContainer.java | 4 +- .../JakartaWebSocketServerContainer.java | 2 +- .../server/JettyWebSocketServerContainer.java | 52 ++++-------- .../tests/JettyWebSocketRestartTest.java | 80 +++++++++++++++++++ .../ee11/servlet/ServletContextHandler.java | 5 ++ .../test/support/XmlBasedJettyServer.java | 3 +- .../JakartaWebSocketClientContainer.java | 4 +- .../JakartaWebSocketServerContainer.java | 2 +- .../server/JettyWebSocketServerContainer.java | 52 ++++-------- .../tests/JettyWebSocketRestartTest.java | 80 +++++++++++++++++++ .../ee9/test/support/XmlBasedJettyServer.java | 3 +- .../JakartaWebSocketClientContainer.java | 4 +- .../JakartaWebSocketServerContainer.java | 2 +- .../ee9/websocket/client/WebSocketClient.java | 2 +- .../server/JettyWebSocketServerContainer.java | 2 +- .../tests/JettyWebSocketRestartTest.java | 2 +- 30 files changed, 279 insertions(+), 141 deletions(-) diff --git a/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/ComponentDocs.java b/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/ComponentDocs.java index 746d8f9aef18..57ac1baab03c 100644 --- a/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/ComponentDocs.java +++ b/documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/ComponentDocs.java @@ -45,7 +45,7 @@ class Root extends ContainerLifeCycle public Root() { // The Monitor life cycle is managed by Root. - addManaged(monitor); + addBeanAndEnsure(this, monitor); } } diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 43e08d41a436..b9179bbf690c 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -516,7 +516,7 @@ public Destination resolveDestination(Origin origin) { HttpDestination newDestination = (HttpDestination)getHttpClientTransport().newDestination(k); // Start the destination before it's published to other threads. - addManaged(newDestination); + addBeanAndEnsure(this, newDestination); if (destinationSweeper != null) destinationSweeper.offer(newDestination); if (LOG.isDebugEnabled()) diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java index 09b41e33f429..1f3b26423b37 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTLSTest.java @@ -67,6 +67,7 @@ import org.eclipse.jetty.toolchain.test.Net; import org.eclipse.jetty.util.Pool; import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.ExecutorThreadPool; import org.eclipse.jetty.util.thread.QueuedThreadPool; @@ -1370,7 +1371,7 @@ public void onClosed(Connection connection) latch.countDown(); } }; - connector.addManaged(serverStats); + ContainerLifeCycle.addBeanAndEnsure(connector, serverStats); SslContextFactory.Client clientTLSFactory = createClientSslContextFactory(); startClient(clientTLSFactory); @@ -1383,7 +1384,7 @@ public void onClosed(Connection connection) latch.countDown(); } }; - client.addManaged(clientStats); + ContainerLifeCycle.addBeanAndEnsure(client, clientStats); ContentResponse response = client.newRequest("https://localhost:" + connector.getLocalPort()) .headers(httpFields -> httpFields.put(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE.asString())) diff --git a/jetty-core/jetty-quic/jetty-quic-quiche/jetty-quic-quiche-client/src/main/java/org/eclipse/jetty/quic/quiche/client/QuicheTransport.java b/jetty-core/jetty-quic/jetty-quic-quiche/jetty-quic-quiche-client/src/main/java/org/eclipse/jetty/quic/quiche/client/QuicheTransport.java index fbcba8c46530..ed89c132bff0 100644 --- a/jetty-core/jetty-quic/jetty-quic-quiche/jetty-quic-quiche-client/src/main/java/org/eclipse/jetty/quic/quiche/client/QuicheTransport.java +++ b/jetty-core/jetty-quic/jetty-quic-quiche/jetty-quic-quiche-client/src/main/java/org/eclipse/jetty/quic/quiche/client/QuicheTransport.java @@ -37,6 +37,7 @@ import org.eclipse.jetty.quic.util.ErrorCode; import org.eclipse.jetty.util.Promise; import org.eclipse.jetty.util.component.Container; +import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -114,7 +115,7 @@ public void onOpen(Session session) { ClientQuicheSession qSession = (ClientQuicheSession)session; ProtocolSession pSession = newProtocolSession(qSession); - qSession.addManaged(pSession); + ContainerLifeCycle.addBeanAndEnsure(qSession, pSession); protocolSession.set(pSession); context.put(ClientConnector.APPLICATION_PROTOCOL_CONTEXT_KEY, qSession.getNegotiatedProtocol()); } diff --git a/jetty-core/jetty-quic/jetty-quic-quiche/jetty-quic-quiche-server/src/main/java/org/eclipse/jetty/quic/quiche/server/QuicheServerConnectionFactory.java b/jetty-core/jetty-quic/jetty-quic-quiche/jetty-quic-quiche-server/src/main/java/org/eclipse/jetty/quic/quiche/server/QuicheServerConnectionFactory.java index 3a57caa91476..983705e9f752 100644 --- a/jetty-core/jetty-quic/jetty-quic-quiche/jetty-quic-quiche-server/src/main/java/org/eclipse/jetty/quic/quiche/server/QuicheServerConnectionFactory.java +++ b/jetty-core/jetty-quic/jetty-quic-quiche/jetty-quic-quiche-server/src/main/java/org/eclipse/jetty/quic/quiche/server/QuicheServerConnectionFactory.java @@ -89,7 +89,7 @@ public void onOpen(Session session) { ServerQuicheSession qSession = (ServerQuicheSession)session; ProtocolSession pSession = newProtocolSession(qSession); - qSession.addManaged(pSession); + addBeanAndEnsure(qSession, pSession); protocolSession.set(pSession); } catch (Throwable x) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index c059e3b7bca3..0cd2f5c932b3 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -1422,7 +1422,7 @@ public void addAliasCheck(AliasCheck check) { _aliasChecks.add(check); if (check instanceof LifeCycle) - addManaged((LifeCycle)check); + addBeanAndEnsure(this, (LifeCycle)check); else addBean(check); } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HotSwapHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HotSwapHandler.java index 89b8db218de0..b65a3eeb311e 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HotSwapHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HotSwapHandler.java @@ -71,7 +71,7 @@ public void setHandler(Handler handler) if (handler != null) { handler.setServer(server); - addManaged(handler); + addBeanAndEnsure(this, handler); } _handler = handler; if (oldHandler != null) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java index c091db334578..cc92c6655062 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java @@ -473,14 +473,30 @@ private boolean installBean(Object o, Managed managed) * wrapped as RuntimeExceptions * * @param lifecycle the managed lifecycle to add + * @deprecated use {@link #addBeanAndEnsure(ContainerLifeCycle, LifeCycle)} instead. */ + @Deprecated(forRemoval = true, since = "jetty-12.1.3") public void addManaged(LifeCycle lifecycle) { - addBean(lifecycle, true); + addBeanAndEnsure(this, lifecycle); + } + + /** + * Adds a managed lifecycle. + *

This is a convenience method that uses addBean(lifecycle,true) + * and then ensures that the added bean is started iff this container + * is running. Exception from nested calls to start are caught and + * wrapped as RuntimeExceptions + * + * @param lifecycle the managed lifecycle to add + */ + public static void addBeanAndEnsure(ContainerLifeCycle container, LifeCycle lifecycle) + { + container.addBean(lifecycle, true); try { - if (isRunning() && !lifecycle.isRunning()) - start(lifecycle); + if (container.isRunning() && !lifecycle.isRunning()) + container.start(lifecycle); } catch (RuntimeException | Error e) { diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketMappings.java b/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketMappings.java index a9e1b8ce5c00..c81fd877821c 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketMappings.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketMappings.java @@ -62,21 +62,22 @@ public static WebSocketMappings ensureMappings(ContextHandler contextHandler) { WebSocketMappings mappings = getMappings(contextHandler); if (mappings == null) + { + mappings = contextHandler.getBean(WebSocketMappings.class); + if (mappings != null) + { + contextHandler.setAttribute(WEBSOCKET_MAPPING_ATTRIBUTE, mappings); + + // The listener only persists through restarts if it is "durable" (if it is added before starting). + if (!contextHandler.getEventListeners().contains(mappings)) + contextHandler.addEventListener(mappings); + } + } + if (mappings == null) { mappings = new WebSocketMappings(WebSocketServerComponents.getWebSocketComponents(contextHandler)); contextHandler.setAttribute(WEBSOCKET_MAPPING_ATTRIBUTE, mappings); contextHandler.addBean(mappings); - WebSocketMappings m = mappings; - contextHandler.addEventListener(new LifeCycle.Listener() - { - @Override - public void lifeCycleStopping(LifeCycle event) - { - contextHandler.removeAttribute(WEBSOCKET_MAPPING_ATTRIBUTE); - contextHandler.removeEventListener(this); - contextHandler.removeBean(m); - } - }); } return mappings; diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java b/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java index b9e88ff39355..d9d30a63fb94 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/WebSocketServerComponents.java @@ -21,7 +21,6 @@ import org.eclipse.jetty.util.Attributes; import org.eclipse.jetty.util.DecoratedObjectFactory; import org.eclipse.jetty.util.component.ContainerLifeCycle; -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; @@ -99,6 +98,13 @@ private static WebSocketComponents ensureWebSocketComponents(Server server, Attr if (components != null) return components; + components = container.getBean(WebSocketServerComponents.class); + if (components != null) + { + attributes.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, components); + return components; + } + InflaterPool inflaterPool = (InflaterPool)attributes.getAttribute(WEBSOCKET_INFLATER_POOL_ATTRIBUTE); if (inflaterPool == null) inflaterPool = InflaterPool.ensurePool(server); @@ -116,44 +122,24 @@ private static WebSocketComponents ensureWebSocketComponents(Server server, Attr executor = server.getThreadPool(); DecoratedObjectFactory objectFactory = (DecoratedObjectFactory)attributes.getAttribute(DecoratedObjectFactory.ATTR); - WebSocketComponents serverComponents = new WebSocketServerComponents(inflaterPool, deflaterPool, bufferPool, objectFactory, executor); + components = new WebSocketServerComponents(inflaterPool, deflaterPool, bufferPool, objectFactory, executor); if (objectFactory != null) - serverComponents.unmanage(objectFactory); + components.unmanage(objectFactory); // 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); + components.unmanage(inflaterPool); if (server.contains(deflaterPool)) - serverComponents.unmanage(deflaterPool); + components.unmanage(deflaterPool); if (server.contains(bufferPool)) - serverComponents.unmanage(bufferPool); + components.unmanage(bufferPool); if (executor != null) - serverComponents.unmanage(executor); + components.unmanage(executor); - // Set to be managed as persistent attribute and bean on ContextHandler. - container.addManaged(serverComponents); - attributes.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, serverComponents); - - // Stop the WebSocketComponents when the ContextHandler stops and remove the WebSocketComponents attribute. - container.addEventListener(new LifeCycle.Listener() - { - @Override - public void lifeCycleStopping(LifeCycle event) - { - attributes.removeAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE); - container.removeBean(serverComponents); - container.removeEventListener(this); - } - - @Override - public String toString() - { - return String.format("%sCleanupListener", WebSocketServerComponents.class.getSimpleName()); - } - }); - - return serverComponents; + addBeanAndEnsure(container, components); + attributes.setAttribute(WEBSOCKET_COMPONENTS_ATTRIBUTE, components); + return components; } public static WebSocketComponents getWebSocketComponents(ContextHandler contextHandler) diff --git a/jetty-core/jetty-websocket/jetty-websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-core/jetty-websocket/jetty-websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index 7dea68d29f57..0ddd5edb88ec 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/jetty-core/jetty-websocket/jetty-websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java @@ -79,7 +79,7 @@ public WebSocketClient() public WebSocketClient(HttpClient httpClient) { coreClient = new WebSocketCoreClient(httpClient, null); - addManaged(coreClient); + addBeanAndEnsure(this, coreClient); frameHandlerFactory = new JettyWebSocketFrameHandlerFactory(this, coreClient.getWebSocketComponents()); sessionListeners.add(sessionTracker); installBean(sessionTracker); diff --git a/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java b/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java index 20f75901b819..d30f6f5d32a5 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java +++ b/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java @@ -83,7 +83,7 @@ public static ServerWebSocketContainer ensure(Server server, ContextHandler cont WebSocketMappings mappings = new WebSocketMappings(components); container = new ServerWebSocketContainer(context, mappings); ContainerLifeCycle parent = contextHandler == null ? server : contextHandler; - parent.addManaged(container); + addBeanAndEnsure(parent, container); } return container; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java index a4970a23e993..032e24ebce74 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextHandler.java @@ -395,6 +395,8 @@ public boolean removeEventListener(EventListener listener) { if (super.removeEventListener(listener)) { + _durableListeners.remove(listener); + if (listener instanceof ServletContextScopeListener) _contextListeners.remove(listener); @@ -911,6 +913,9 @@ public boolean addEventListener(EventListener listener) { if (super.addEventListener(listener)) { + if (isRunning() && contains(listener)) + _durableListeners.add(listener); + if ((listener instanceof HttpSessionActivationListener) || (listener instanceof HttpSessionAttributeListener) || (listener instanceof HttpSessionBindingListener) || diff --git a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/support/XmlBasedJettyServer.java b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/support/XmlBasedJettyServer.java index e790893bfa08..3033e9ec5a64 100644 --- a/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/support/XmlBasedJettyServer.java +++ b/jetty-ee10/jetty-ee10-tests/jetty-ee10-test-integration/src/test/java/org/eclipse/jetty/ee10/test/support/XmlBasedJettyServer.java @@ -28,6 +28,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.toolchain.test.MavenPaths; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; +import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; @@ -151,7 +152,7 @@ public void load() throws Exception public void start() throws Exception { - _server.addManaged(_resourceFactory); + ContainerLifeCycle.addBeanAndEnsure(_server, _resourceFactory); _server.start(); } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientContainer.java index e9773e4a9153..58a33368592c 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientContainer.java @@ -116,7 +116,7 @@ protected WebSocketCoreClient getWebSocketCoreClient() if (coreClient == null) { coreClient = coreClientFactory.apply(components); - addManaged(coreClient); + addBeanAndEnsure(this, coreClient); } return coreClient; @@ -332,7 +332,7 @@ protected void doClientStart() ContainerLifeCycle container = SHUTDOWN_MAP.get(cl); if (container != null) { - container.addManaged(this); + addBeanAndEnsure(container, this); if (LOG.isDebugEnabled()) LOG.debug("{} registered for WebApp shutdown to {}", this, container); return; diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java index ffd856ffd802..2ab5d2926e3e 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java @@ -111,7 +111,7 @@ public static JakartaWebSocketServerContainer ensureContainer(ServletContext ser coreClientSupplier); // Manage the lifecycle of the Container. - contextHandler.addManaged(container); + addBeanAndEnsure(contextHandler, container); contextHandler.addEventListener(container); contextHandler.addEventListener(new LifeCycle.Listener() { diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServerContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServerContainer.java index 0ae85259f88e..2c96782de13c 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServerContainer.java @@ -72,41 +72,22 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl throw new IllegalStateException("Server has not been set on the ServletContextHandler"); // If we find a container in the servlet context return it. - JettyWebSocketServerContainer containerFromServletContext = getContainer(servletContext); - if (containerFromServletContext != null) - return containerFromServletContext; + JettyWebSocketServerContainer container = getContainer(servletContext); + if (container != null) + return container; - // Find Pre-Existing executor. - Executor executor = (Executor)servletContext.getAttribute("org.eclipse.jetty.server.Executor"); - if (executor == null) - executor = contextHandler.getServer().getThreadPool(); + container = contextHandler.getBean(JettyWebSocketServerContainer.class); + if (container != null) + { + servletContext.setAttribute(JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE, container); + return container; + } // Create the Jetty ServerContainer implementation. WebSocketMappings mappings = WebSocketMappings.ensureMappings(contextHandler); WebSocketComponents components = WebSocketServerComponents.getWebSocketComponents(contextHandler); - JettyWebSocketServerContainer container = new JettyWebSocketServerContainer(contextHandler, mappings, components, executor); - - // Manage the lifecycle of the Container. - contextHandler.addManaged(container); - contextHandler.addEventListener(container); - contextHandler.addEventListener(new LifeCycle.Listener() - { - @Override - public void lifeCycleStopping(LifeCycle event) - { - contextHandler.getServletContext().removeAttribute(JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE); - contextHandler.removeBean(container); - contextHandler.removeEventListener(container); - contextHandler.removeEventListener(this); - } - - @Override - public String toString() - { - return String.format("%sCleanupListener", JettyWebSocketServerContainer.class.getSimpleName()); - } - }); - + container = new JettyWebSocketServerContainer(contextHandler, mappings, components); + addBeanAndEnsure(contextHandler, container); servletContext.setAttribute(JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE, container); return container; } @@ -117,7 +98,6 @@ public String toString() private final WebSocketMappings webSocketMappings; private final WebSocketComponents components; private final JettyServerFrameHandlerFactory frameHandlerFactory; - private final Executor executor; private final Configuration.ConfigurationCustomizer customizer = new Configuration.ConfigurationCustomizer(); private final List sessionListeners = new ArrayList<>(); @@ -127,18 +107,18 @@ public String toString() * Main entry point for {@link JettyWebSocketServletContainerInitializer}. * * @param webSocketMappings the {@link WebSocketMappings} that this container belongs to - * @param executor the {@link Executor} to use */ - JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMappings webSocketMappings, WebSocketComponents components, Executor executor) + JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMappings webSocketMappings, WebSocketComponents components) { this.contextHandler = contextHandler; this.webSocketMappings = webSocketMappings; this.components = components; - this.executor = executor; this.frameHandlerFactory = new JettyServerFrameHandlerFactory(this, components); - installBean(frameHandlerFactory); addSessionListener(sessionTracker); + installBean(webSocketMappings); + installBean(components); + installBean(frameHandlerFactory); installBean(sessionTracker); } @@ -247,7 +227,7 @@ public boolean upgrade(JettyWebSocketCreator creator, HttpServletRequest request @Override public Executor getExecutor() { - return this.executor; + return this.components.getExecutor(); } @Override diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketRestartTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketRestartTest.java index 05bfdecffab3..d90c8fc9a3e2 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketRestartTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketRestartTest.java @@ -24,17 +24,23 @@ import org.eclipse.jetty.ee10.websocket.servlet.WebSocketUpgradeFilter; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.websocket.api.Callback; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.client.WebSocketClient; +import org.eclipse.jetty.websocket.core.WebSocketComponents; +import org.eclipse.jetty.websocket.core.server.WebSocketMappings; 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 org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -105,6 +111,80 @@ public void testWebSocketRestart() throws Exception assertThat(contextHandler.getServletHandler().getFilters().length, is(0)); } + @ParameterizedTest + @ValueSource(booleans = {false, true}) + public void testContextRestart(boolean earlyInitWebSocketComponents) throws Exception + { + if (earlyInitWebSocketComponents) + WebSocketServerComponents.ensureWebSocketComponents(server, contextHandler); + JettyWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> + container.addMapping("/", EchoSocket.class)); + server.start(); + WebSocketComponents components = WebSocketServerComponents.getWebSocketComponents(contextHandler); + + int initialNumEventListeners = contextHandler.getEventListeners().size(); + for (int i = 0; i < 100; i++) + { + assertThat(contextHandler.getEventListeners().size(), is(initialNumEventListeners)); + assertThat(contextHandler.getContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE), sameInstance(components)); + assertThat(contextHandler.getBean(WebSocketServerComponents.class), sameInstance(components)); + + contextHandler.stop(); + + // The CompressionPools should still be running because they are a resource managed by the server. + assertTrue(components.getDeflaterPool().isRunning()); + assertTrue(components.getInflaterPool().isRunning()); + + // Even though the components is now stopped the executor is still running as it has been taken from the server. + assertTrue(components.isStopped()); + assertThat(components.getExecutor(), instanceOf(QueuedThreadPool.class)); + assertTrue(((QueuedThreadPool)components.getExecutor()).isRunning()); + + // The components now persists as a bean though restarts. + assertNull(contextHandler.getContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE)); + assertThat(contextHandler.getBean(WebSocketServerComponents.class), sameInstance(components)); + + contextHandler.start(); + testEchoMessage(); + } + + // Verify we have not accumulated websocket resources by restarting. + assertThat(contextHandler.getEventListeners().size(), is(initialNumEventListeners)); + assertThat(contextHandler.getContainedBeans(JettyWebSocketServerContainer.class).size(), is(1)); + assertThat(contextHandler.getContainedBeans(WebSocketServerComponents.class).size(), is(1)); + assertNotNull(contextHandler.getServletContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE)); + assertNotNull(contextHandler.getServletContext().getAttribute(JettyWebSocketServerContainer.JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE)); + + // We have one filter, and it is a WebSocketUpgradeFilter. + FilterHolder[] filters = contextHandler.getServletHandler().getFilters(); + assertThat(filters.length, is(1)); + assertThat(filters[0].getFilter(), instanceOf(WebSocketUpgradeFilter.class)); + + // Verify the state after stopping the server. + contextHandler.stop(); + assertThat(contextHandler.getEventListeners().size(), is(2)); + + // Server managed components should still be running. + assertThat(contextHandler.getContainedBeans(WebSocketServerComponents.class).size(), is(1)); + assertThat(contextHandler.getBean(WebSocketServerComponents.class), sameInstance(components)); + assertTrue(components.getInflaterPool().isRunning()); + assertTrue(components.getDeflaterPool().isRunning()); + assertTrue(((QueuedThreadPool)components.getExecutor()).isRunning()); + + // The other components should now be stopped. + assertThat(contextHandler.getContainedBeans(JettyWebSocketServerContainer.class).size(), is(1)); + assertThat(contextHandler.getContainedBeans(WebSocketMappings.class).size(), is(1)); + assertTrue(contextHandler.getBean(JettyWebSocketServerContainer.class).isStopped()); + assertTrue(components.isStopped()); + + // Attributes should be removed. + assertNull(contextHandler.getServletContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE)); + assertNull(contextHandler.getServletContext().getAttribute(JettyWebSocketServerContainer.JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE)); + + // The WebSocketUpgradeFilter should be removed. + assertThat(contextHandler.getServletHandler().getFilters().length, is(0)); + } + private void testEchoMessage() throws Exception { // Test we can upgrade to websocket and send a message. diff --git a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletContextHandler.java b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletContextHandler.java index 24f20d9965d6..b2c367621107 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletContextHandler.java +++ b/jetty-ee11/jetty-ee11-servlet/src/main/java/org/eclipse/jetty/ee11/servlet/ServletContextHandler.java @@ -392,6 +392,8 @@ public boolean removeEventListener(EventListener listener) { if (super.removeEventListener(listener)) { + _durableListeners.remove(listener); + if (listener instanceof ServletContextScopeListener) _contextListeners.remove(listener); @@ -907,6 +909,9 @@ public boolean addEventListener(EventListener listener) { if (super.addEventListener(listener)) { + if (isRunning() && contains(listener)) + _durableListeners.add(listener); + if ((listener instanceof HttpSessionActivationListener) || (listener instanceof HttpSessionAttributeListener) || (listener instanceof HttpSessionBindingListener) || diff --git a/jetty-ee11/jetty-ee11-tests/jetty-ee11-test-integration/src/test/java/org/eclipse/jetty/ee11/test/support/XmlBasedJettyServer.java b/jetty-ee11/jetty-ee11-tests/jetty-ee11-test-integration/src/test/java/org/eclipse/jetty/ee11/test/support/XmlBasedJettyServer.java index 5fd5fb9a0cb9..73b5c89fa12d 100644 --- a/jetty-ee11/jetty-ee11-tests/jetty-ee11-test-integration/src/test/java/org/eclipse/jetty/ee11/test/support/XmlBasedJettyServer.java +++ b/jetty-ee11/jetty-ee11-tests/jetty-ee11-test-integration/src/test/java/org/eclipse/jetty/ee11/test/support/XmlBasedJettyServer.java @@ -28,6 +28,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.toolchain.test.MavenPaths; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; +import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; @@ -151,7 +152,7 @@ public void load() throws Exception public void start() throws Exception { - _server.addManaged(_resourceFactory); + ContainerLifeCycle.addBeanAndEnsure(_server, _resourceFactory); _server.start(); } diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/client/JakartaWebSocketClientContainer.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/client/JakartaWebSocketClientContainer.java index d0643250de79..a2cc6b3eaf0f 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/client/JakartaWebSocketClientContainer.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/client/JakartaWebSocketClientContainer.java @@ -116,7 +116,7 @@ protected WebSocketCoreClient getWebSocketCoreClient() if (coreClient == null) { coreClient = coreClientFactory.apply(components); - addManaged(coreClient); + addBeanAndEnsure(this, coreClient); } return coreClient; @@ -327,7 +327,7 @@ protected void doClientStart() ContainerLifeCycle container = SHUTDOWN_MAP.get(cl); if (container != null) { - container.addManaged(this); + addBeanAndEnsure(container, this); if (LOG.isDebugEnabled()) LOG.debug("{} registered for WebApp shutdown to {}", this, container); return; diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/server/JakartaWebSocketServerContainer.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/server/JakartaWebSocketServerContainer.java index 870b3a910013..3c4c16936ff3 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/server/JakartaWebSocketServerContainer.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/server/JakartaWebSocketServerContainer.java @@ -111,7 +111,7 @@ public static JakartaWebSocketServerContainer ensureContainer(ServletContext ser coreClientSupplier); // Manage the lifecycle of the Container. - contextHandler.addManaged(container); + addBeanAndEnsure(contextHandler, container); contextHandler.addEventListener(container); contextHandler.addEventListener(new LifeCycle.Listener() { diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee11/websocket/server/JettyWebSocketServerContainer.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee11/websocket/server/JettyWebSocketServerContainer.java index 145092a32838..25cd85453f64 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee11/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee11/websocket/server/JettyWebSocketServerContainer.java @@ -72,41 +72,22 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl throw new IllegalStateException("Server has not been set on the ServletContextHandler"); // If we find a container in the servlet context return it. - JettyWebSocketServerContainer containerFromServletContext = getContainer(servletContext); - if (containerFromServletContext != null) - return containerFromServletContext; + JettyWebSocketServerContainer container = getContainer(servletContext); + if (container != null) + return container; - // Find Pre-Existing executor. - Executor executor = (Executor)servletContext.getAttribute("org.eclipse.jetty.server.Executor"); - if (executor == null) - executor = contextHandler.getServer().getThreadPool(); + container = contextHandler.getBean(JettyWebSocketServerContainer.class); + if (container != null) + { + servletContext.setAttribute(JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE, container); + return container; + } // Create the Jetty ServerContainer implementation. WebSocketMappings mappings = WebSocketMappings.ensureMappings(contextHandler); WebSocketComponents components = WebSocketServerComponents.getWebSocketComponents(contextHandler); - JettyWebSocketServerContainer container = new JettyWebSocketServerContainer(contextHandler, mappings, components, executor); - - // Manage the lifecycle of the Container. - contextHandler.addManaged(container); - contextHandler.addEventListener(container); - contextHandler.addEventListener(new LifeCycle.Listener() - { - @Override - public void lifeCycleStopping(LifeCycle event) - { - contextHandler.getServletContext().removeAttribute(JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE); - contextHandler.removeBean(container); - contextHandler.removeEventListener(container); - contextHandler.removeEventListener(this); - } - - @Override - public String toString() - { - return String.format("%sCleanupListener", JettyWebSocketServerContainer.class.getSimpleName()); - } - }); - + container = new JettyWebSocketServerContainer(contextHandler, mappings, components); + addBeanAndEnsure(contextHandler, container); servletContext.setAttribute(JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE, container); return container; } @@ -117,7 +98,6 @@ public String toString() private final WebSocketMappings webSocketMappings; private final WebSocketComponents components; private final JettyServerFrameHandlerFactory frameHandlerFactory; - private final Executor executor; private final Configuration.ConfigurationCustomizer customizer = new Configuration.ConfigurationCustomizer(); private final List sessionListeners = new ArrayList<>(); @@ -127,18 +107,18 @@ public String toString() * Main entry point for {@link JettyWebSocketServletContainerInitializer}. * * @param webSocketMappings the {@link WebSocketMappings} that this container belongs to - * @param executor the {@link Executor} to use */ - JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMappings webSocketMappings, WebSocketComponents components, Executor executor) + JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMappings webSocketMappings, WebSocketComponents components) { this.contextHandler = contextHandler; this.webSocketMappings = webSocketMappings; this.components = components; - this.executor = executor; this.frameHandlerFactory = new JettyServerFrameHandlerFactory(this, components); - installBean(frameHandlerFactory); addSessionListener(sessionTracker); + installBean(webSocketMappings); + installBean(components); + installBean(frameHandlerFactory); installBean(sessionTracker); } @@ -247,7 +227,7 @@ public boolean upgrade(JettyWebSocketCreator creator, HttpServletRequest request @Override public Executor getExecutor() { - return this.executor; + return this.components.getExecutor(); } @Override diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/JettyWebSocketRestartTest.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/JettyWebSocketRestartTest.java index cadf7c5f064b..9aaaa11198cf 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/JettyWebSocketRestartTest.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/JettyWebSocketRestartTest.java @@ -24,17 +24,23 @@ import org.eclipse.jetty.ee11.websocket.servlet.WebSocketUpgradeFilter; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.util.thread.QueuedThreadPool; import org.eclipse.jetty.websocket.api.Callback; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.client.WebSocketClient; +import org.eclipse.jetty.websocket.core.WebSocketComponents; +import org.eclipse.jetty.websocket.core.server.WebSocketMappings; 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 org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -105,6 +111,80 @@ public void testWebSocketRestart() throws Exception assertThat(contextHandler.getServletHandler().getFilters().length, is(0)); } + @ParameterizedTest + @ValueSource(booleans = {false, true}) + public void testContextRestart(boolean earlyInitWebSocketComponents) throws Exception + { + if (earlyInitWebSocketComponents) + WebSocketServerComponents.ensureWebSocketComponents(server, contextHandler); + JettyWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> + container.addMapping("/", EchoSocket.class)); + server.start(); + WebSocketComponents components = WebSocketServerComponents.getWebSocketComponents(contextHandler); + + int initialNumEventListeners = contextHandler.getEventListeners().size(); + for (int i = 0; i < 100; i++) + { + assertThat(contextHandler.getEventListeners().size(), is(initialNumEventListeners)); + assertThat(contextHandler.getContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE), sameInstance(components)); + assertThat(contextHandler.getBean(WebSocketServerComponents.class), sameInstance(components)); + + contextHandler.stop(); + + // The CompressionPools should still be running because they are a resource managed by the server. + assertTrue(components.getDeflaterPool().isRunning()); + assertTrue(components.getInflaterPool().isRunning()); + + // Even though the components is now stopped the executor is still running as it has been taken from the server. + assertTrue(components.isStopped()); + assertThat(components.getExecutor(), instanceOf(QueuedThreadPool.class)); + assertTrue(((QueuedThreadPool)components.getExecutor()).isRunning()); + + // The components now persists as a bean though restarts. + assertNull(contextHandler.getContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE)); + assertThat(contextHandler.getBean(WebSocketServerComponents.class), sameInstance(components)); + + contextHandler.start(); + testEchoMessage(); + } + + // Verify we have not accumulated websocket resources by restarting. + assertThat(contextHandler.getEventListeners().size(), is(initialNumEventListeners)); + assertThat(contextHandler.getContainedBeans(JettyWebSocketServerContainer.class).size(), is(1)); + assertThat(contextHandler.getContainedBeans(WebSocketServerComponents.class).size(), is(1)); + assertNotNull(contextHandler.getServletContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE)); + assertNotNull(contextHandler.getServletContext().getAttribute(JettyWebSocketServerContainer.JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE)); + + // We have one filter, and it is a WebSocketUpgradeFilter. + FilterHolder[] filters = contextHandler.getServletHandler().getFilters(); + assertThat(filters.length, is(1)); + assertThat(filters[0].getFilter(), instanceOf(WebSocketUpgradeFilter.class)); + + // Verify the state after stopping the server. + contextHandler.stop(); + assertThat(contextHandler.getEventListeners().size(), is(2)); + + // Server managed components should still be running. + assertThat(contextHandler.getContainedBeans(WebSocketServerComponents.class).size(), is(1)); + assertThat(contextHandler.getBean(WebSocketServerComponents.class), sameInstance(components)); + assertTrue(components.getInflaterPool().isRunning()); + assertTrue(components.getDeflaterPool().isRunning()); + assertTrue(((QueuedThreadPool)components.getExecutor()).isRunning()); + + // The other components should now be stopped. + assertThat(contextHandler.getContainedBeans(JettyWebSocketServerContainer.class).size(), is(1)); + assertThat(contextHandler.getContainedBeans(WebSocketMappings.class).size(), is(1)); + assertTrue(contextHandler.getBean(JettyWebSocketServerContainer.class).isStopped()); + assertTrue(components.isStopped()); + + // Attributes should be removed. + assertNull(contextHandler.getServletContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE)); + assertNull(contextHandler.getServletContext().getAttribute(JettyWebSocketServerContainer.JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE)); + + // The WebSocketUpgradeFilter should be removed. + assertThat(contextHandler.getServletHandler().getFilters().length, is(0)); + } + private void testEchoMessage() throws Exception { // Test we can upgrade to websocket and send a message. diff --git a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/support/XmlBasedJettyServer.java b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/support/XmlBasedJettyServer.java index f87a6a892924..6e7583edd855 100644 --- a/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/support/XmlBasedJettyServer.java +++ b/jetty-ee9/jetty-ee9-tests/jetty-ee9-test-integration/src/test/java/org/eclipse/jetty/ee9/test/support/XmlBasedJettyServer.java @@ -28,6 +28,7 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.toolchain.test.MavenPaths; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; +import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceFactory; @@ -151,7 +152,7 @@ public void load() throws Exception public void start() throws Exception { - _server.addManaged(_resourceFactory); + ContainerLifeCycle.addBeanAndEnsure(_server, _resourceFactory); _server.start(); } diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/client/JakartaWebSocketClientContainer.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/client/JakartaWebSocketClientContainer.java index 1180bb30dce6..3218453c6267 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/client/JakartaWebSocketClientContainer.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/client/JakartaWebSocketClientContainer.java @@ -116,7 +116,7 @@ protected WebSocketCoreClient getWebSocketCoreClient() if (coreClient == null) { coreClient = coreClientFactory.apply(components); - addManaged(coreClient); + addBeanAndEnsure(this, coreClient); } return coreClient; @@ -332,7 +332,7 @@ protected void doClientStart() ContainerLifeCycle container = SHUTDOWN_MAP.get(cl); if (container != null) { - container.addManaged(this); + addBeanAndEnsure(container, this); if (LOG.isDebugEnabled()) LOG.debug("{} registered for Context shutdown to {}", this, container); return; diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/JakartaWebSocketServerContainer.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/JakartaWebSocketServerContainer.java index a0e2540450a0..1afcc430d40b 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/JakartaWebSocketServerContainer.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/JakartaWebSocketServerContainer.java @@ -110,7 +110,7 @@ public static JakartaWebSocketServerContainer ensureContainer(ServletContext ser coreClientSupplier); // Manage the lifecycle of the Container. - contextHandler.addManaged(container); + addBeanAndEnsure(contextHandler, container); contextHandler.addEventListener(container); contextHandler.addEventListener(new LifeCycle.Listener() { diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-client/src/main/java/org/eclipse/jetty/ee9/websocket/client/WebSocketClient.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-client/src/main/java/org/eclipse/jetty/ee9/websocket/client/WebSocketClient.java index 0878f8e5a914..38d0093abe19 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-client/src/main/java/org/eclipse/jetty/ee9/websocket/client/WebSocketClient.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-client/src/main/java/org/eclipse/jetty/ee9/websocket/client/WebSocketClient.java @@ -82,7 +82,7 @@ public WebSocketClient() public WebSocketClient(HttpClient httpClient) { coreClient = new WebSocketCoreClient(httpClient, components); - addManaged(coreClient); + addBeanAndEnsure(this, coreClient); frameHandlerFactory = new JettyWebSocketFrameHandlerFactory(this, components); sessionListeners.add(sessionTracker); installBean(sessionTracker); diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServerContainer.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServerContainer.java index 245fc7855641..f306dbe788ac 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServerContainer.java @@ -88,7 +88,7 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl JettyWebSocketServerContainer container = new JettyWebSocketServerContainer(contextHandler, mappings, components, executor); // Manage the lifecycle of the Container. - contextHandler.addManaged(container); + addBeanAndEnsure(contextHandler, container); contextHandler.addEventListener(container); contextHandler.addEventListener(new LifeCycle.Listener() { diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee9/websocket/tests/JettyWebSocketRestartTest.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee9/websocket/tests/JettyWebSocketRestartTest.java index ebd9248e5603..8297acabb545 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee9/websocket/tests/JettyWebSocketRestartTest.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee9/websocket/tests/JettyWebSocketRestartTest.java @@ -98,7 +98,7 @@ public void testWebSocketRestart() throws Exception server.stop(); assertThat(contextHandler.getEventListeners().size(), is(0)); assertThat(contextHandler.getContainedBeans(JettyWebSocketServerContainer.class).size(), is(0)); - assertThat(contextHandler.getCoreContextHandler().getContainedBeans(WebSocketServerComponents.class).size(), is(0)); + assertThat(contextHandler.getCoreContextHandler().getContainedBeans(WebSocketServerComponents.class).size(), is(1)); assertNull(contextHandler.getServletContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE)); assertNull(contextHandler.getServletContext().getAttribute(JettyWebSocketServerContainer.JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE)); assertThat(contextHandler.getServletHandler().getFilters().length, is(0)); From d875c4f670bf55dc4dbe72c46809d911fc8cf438 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 10 Oct 2025 14:40:40 +1100 Subject: [PATCH 2/2] Review the lifecycle of WebSocket beans Signed-off-by: Lachlan Roberts --- .../jetty/websocket/server/ServerWebSocketContainer.java | 7 ++++--- .../jakarta/common/JakartaWebSocketContainer.java | 1 + .../ee10/websocket/tests/JettyWebSocketRestartTest.java | 6 +++--- .../jakarta/common/JakartaWebSocketContainer.java | 1 + .../ee11/websocket/tests/JettyWebSocketRestartTest.java | 6 +++--- .../jakarta/common/JakartaWebSocketContainer.java | 1 + 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java b/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java index d30f6f5d32a5..86e6f997a42b 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java +++ b/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java @@ -81,7 +81,7 @@ public static ServerWebSocketContainer ensure(Server server, ContextHandler cont ? WebSocketServerComponents.ensureWebSocketComponents(server) : WebSocketServerComponents.ensureWebSocketComponents(server, contextHandler); WebSocketMappings mappings = new WebSocketMappings(components); - container = new ServerWebSocketContainer(context, mappings); + container = new ServerWebSocketContainer(context, components, mappings); ContainerLifeCycle parent = contextHandler == null ? server : contextHandler; addBeanAndEnsure(parent, container); } @@ -127,13 +127,14 @@ public static ServerWebSocketContainer get(Context context) private final FrameHandlerFactory factory; private InvocationType invocationType = InvocationType.BLOCKING; - ServerWebSocketContainer(Context context, WebSocketMappings mappings) + ServerWebSocketContainer(Context context, WebSocketComponents components, WebSocketMappings mappings) { this.context = context; this.mappings = mappings; - installBean(mappings); this.factory = new ServerFrameHandlerFactory(this, mappings.getWebSocketComponents()); addSessionListener(sessionTracker); + installBean(components); + installBean(mappings); installBean(sessionTracker); } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketContainer.java index 68322562896c..530c99a3c98e 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketContainer.java @@ -48,6 +48,7 @@ public JakartaWebSocketContainer(WebSocketComponents components) this.components = components; addSessionListener(sessionTracker); installBean(sessionTracker); + installBean(components); } public abstract Executor getExecutor(); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketRestartTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketRestartTest.java index d90c8fc9a3e2..15b66d83b566 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketRestartTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee10/websocket/tests/JettyWebSocketRestartTest.java @@ -103,9 +103,9 @@ public void testWebSocketRestart() throws Exception // After stopping the websocket resources are cleaned up. server.stop(); - assertThat(contextHandler.getEventListeners().size(), is(0)); - assertThat(contextHandler.getContainedBeans(JettyWebSocketServerContainer.class).size(), is(0)); - assertThat(contextHandler.getContainedBeans(WebSocketServerComponents.class).size(), is(0)); + assertThat(contextHandler.getEventListeners().size(), is(2)); + assertThat(contextHandler.getContainedBeans(JettyWebSocketServerContainer.class).size(), is(1)); + assertThat(contextHandler.getContainedBeans(WebSocketServerComponents.class).size(), is(1)); assertNull(contextHandler.getServletContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE)); assertNull(contextHandler.getServletContext().getAttribute(JettyWebSocketServerContainer.JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE)); assertThat(contextHandler.getServletHandler().getFilters().length, is(0)); diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/JakartaWebSocketContainer.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/JakartaWebSocketContainer.java index 110271378b8c..fe95b86dfd19 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/JakartaWebSocketContainer.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/JakartaWebSocketContainer.java @@ -49,6 +49,7 @@ public JakartaWebSocketContainer(WebSocketComponents components) this.components = components; addSessionListener(sessionTracker); installBean(sessionTracker); + installBean(components); } public abstract Executor getExecutor(); diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/JettyWebSocketRestartTest.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/JettyWebSocketRestartTest.java index 9aaaa11198cf..bc298641b8e9 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/JettyWebSocketRestartTest.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee11/websocket/tests/JettyWebSocketRestartTest.java @@ -103,9 +103,9 @@ public void testWebSocketRestart() throws Exception // After stopping the websocket resources are cleaned up. server.stop(); - assertThat(contextHandler.getEventListeners().size(), is(0)); - assertThat(contextHandler.getContainedBeans(JettyWebSocketServerContainer.class).size(), is(0)); - assertThat(contextHandler.getContainedBeans(WebSocketServerComponents.class).size(), is(0)); + assertThat(contextHandler.getEventListeners().size(), is(2)); + assertThat(contextHandler.getContainedBeans(JettyWebSocketServerContainer.class).size(), is(1)); + assertThat(contextHandler.getContainedBeans(WebSocketServerComponents.class).size(), is(1)); assertNull(contextHandler.getServletContext().getAttribute(WebSocketServerComponents.WEBSOCKET_COMPONENTS_ATTRIBUTE)); assertNull(contextHandler.getServletContext().getAttribute(JettyWebSocketServerContainer.JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE)); assertThat(contextHandler.getServletHandler().getFilters().length, is(0)); diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/common/JakartaWebSocketContainer.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/common/JakartaWebSocketContainer.java index f64b1bbf9615..1e86446efdee 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/common/JakartaWebSocketContainer.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/common/JakartaWebSocketContainer.java @@ -48,6 +48,7 @@ public JakartaWebSocketContainer(WebSocketComponents components) this.components = components; addSessionListener(sessionTracker); installBean(sessionTracker); + installBean(components); } public abstract Executor getExecutor();