This is an automated email from the ASF dual-hosted git repository.
sergehuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/master by this push:
new a456a6a28 Stop the GraphQL WebSocket deadline scheduler when the
servlet is destroyed (#858)
a456a6a28 is described below
commit a456a6a28b3360cc131d025b4801946e93b92d2a
Author: Serge Huber <[email protected]>
AuthorDate: Thu Sep 17 11:55:34 2026 +0200
Stop the GraphQL WebSocket deadline scheduler when the servlet is destroyed
(#858)
Co-authored-by: Claude Opus 4.8 <[email protected]>
---
.../unomi/graphql/servlet/GraphQLServlet.java | 22 ++++++++++++++++++++--
.../websocket/SubscriptionWebSocketFactory.java | 20 +++++++++++---------
.../unomi/graphql/servlet/GraphQLServletTest.java | 21 +++++++++++++++++++++
.../SubscriptionWebSocketFactoryTest.java | 11 +++++++++++
4 files changed, 63 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..e4b3f88a8 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,34 @@ public class GraphQLServlet extends WebSocketServlet {
private WebSocketServletFactory factory;
+ private SubscriptionWebSocketFactory socketCreator;
+
+ /** For tests: the creator whose scheduler {@link #destroy()} must stop. */
+ SubscriptionWebSocketFactory socketCreator() {
+ return 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..9dc5cc848 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,13 @@ 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();
+ }
+
+ /** Whether {@link #shutdown()} has run; lets the servlet test verify the
destroy() wiring. */
+ public boolean isShutdown() {
+ return authenticationDeadlineScheduler.isShutdown();
}
}
diff --git
a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/GraphQLServletTest.java
b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/GraphQLServletTest.java
index ce8f62904..a6dab3341 100644
---
a/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/GraphQLServletTest.java
+++
b/graphql/cxs-impl/src/test/java/org/apache/unomi/graphql/servlet/GraphQLServletTest.java
@@ -21,6 +21,8 @@ import org.apache.unomi.api.security.SecurityService;
import org.apache.unomi.api.services.ExecutionContextManager;
import org.apache.unomi.graphql.servlet.auth.GraphQLServletSecurityValidator;
import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
+import org.apache.unomi.graphql.schema.GraphQLSchemaUpdater;
+import org.eclipse.jetty.websocket.api.WebSocketPolicy;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -38,6 +40,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
@@ -261,4 +265,21 @@ class GraphQLServletTest {
nonUpgradeCalled.set(true);
}
}
+
+ /**
+ * The creator is only ever Jetty's WebSocketCreator, never a started
lifecycle, so its scheduler is
+ * stopped from destroy(). This pins the wiring itself, not just that
shutdown() works in isolation.
+ */
+ @Test
+ void destroy_shutsDownTheSocketCreatorScheduler() {
+ servlet.setGraphQLSchemaUpdater(mock(GraphQLSchemaUpdater.class));
+ when(factory.getPolicy()).thenReturn(mock(WebSocketPolicy.class));
+ servlet.configure(factory);
+ assertNotNull(servlet.socketCreator());
+ assertFalse(servlet.socketCreator().isShutdown());
+
+ servlet.destroy();
+
+ assertTrue(servlet.socketCreator().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());
+ }
}
