This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch graphql-ws-scheduler-shutdown in repository https://gitbox.apache.org/repos/asf/unomi.git
commit d8a0d41266200a3ee89d2e4e137152fd7081afb2 Author: Serge Huber <[email protected]> AuthorDate: Fri Sep 4 17:17:55 2026 +0200 Stop the GraphQL WebSocket deadline scheduler when the servlet is destroyed SubscriptionWebSocketFactory is only ever handed to Jetty as the socket creator; it is never started or stopped as a lifecycle, so the doStop() override that was meant to shut its scheduler down never ran and the thread outlived each servlet. Shut it down explicitly from GraphQLServlet.destroy() instead, and cover it with a test. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../apache/unomi/graphql/servlet/GraphQLServlet.java | 17 +++++++++++++++-- .../websocket/SubscriptionWebSocketFactory.java | 19 ++++++++++--------- .../websocket/SubscriptionWebSocketFactoryTest.java | 11 +++++++++++ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/GraphQLServlet.java b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/GraphQLServlet.java index 3443caca8..307a93baf 100644 --- a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/GraphQLServlet.java +++ b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/GraphQLServlet.java @@ -108,16 +108,29 @@ public class GraphQLServlet extends WebSocketServlet { private WebSocketServletFactory factory; + private SubscriptionWebSocketFactory socketCreator; + + @Override + public void destroy() { + try { + if (socketCreator != null) { + socketCreator.shutdown(); + } + } finally { + super.destroy(); + } + } + @Override public void configure(WebSocketServletFactory factory) { LOGGER.debug("GraphQLServlet configured"); this.factory = factory; // Wrap the WebSocket creator to bind the authenticated subject established during upgrade - SubscriptionWebSocketFactory originalCreator = new SubscriptionWebSocketFactory( + this.socketCreator = new SubscriptionWebSocketFactory( graphQLSchemaUpdater.getGraphQL(), serviceManager, securityService, executionContextManager, validator); factory.setCreator((req, resp) -> { try { - return originalCreator.createWebSocket(req, resp); + return socketCreator.createWebSocket(req, resp); } finally { cleanupSecurityContext(); } diff --git a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactory.java b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactory.java index 0d0c949da..3ce1c4589 100644 --- a/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactory.java +++ b/graphql/cxs-impl/src/main/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactory.java @@ -44,8 +44,9 @@ public class SubscriptionWebSocketFactory extends WebSocketServerFactory { private final GraphQLServletSecurityValidator validator; /** - * Closes sockets that do not authenticate within their deadline. One daemon thread for all sockets; - * stopped with the factory, which {@code WebSocketServlet.destroy()} stops on undeploy. + * Closes sockets that do not authenticate within their deadline. One daemon thread for all sockets. + * This object is only ever Jetty's creator, never a started lifecycle, so the servlet shuts the + * scheduler down explicitly from {@code destroy()}. */ private final ScheduledExecutorService authenticationDeadlineScheduler; @@ -76,12 +77,12 @@ public class SubscriptionWebSocketFactory extends WebSocketServerFactory { securityService, executionContextManager, validator, authenticationDeadlineScheduler); } - @Override - protected void doStop() throws Exception { - try { - super.doStop(); - } finally { - authenticationDeadlineScheduler.shutdownNow(); - } + /** Stops the deadline scheduler; called when the owning servlet is destroyed. */ + public void shutdown() { + authenticationDeadlineScheduler.shutdownNow(); + } + + boolean isShutdown() { + return authenticationDeadlineScheduler.isShutdown(); } } diff --git a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactoryTest.java b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactoryTest.java index 84e58b1ca..b74e7f351 100644 --- a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactoryTest.java +++ b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/websocket/SubscriptionWebSocketFactoryTest.java @@ -33,6 +33,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import javax.security.auth.Subject; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -91,4 +92,14 @@ class SubscriptionWebSocketFactoryTest { assertTrue(socket instanceof SubscriptionWebSocket); verify(upgradeResponse, never()).setStatusCode(401); } + + /** The creator is never a started Jetty lifecycle, so its scheduler must be stopped explicitly. */ + @Test + void shutdown_stopsTheDeadlineScheduler() { + assertFalse(factory.isShutdown()); + + factory.shutdown(); + + assertTrue(factory.isShutdown()); + } }
