From 753c8be6ebe63fe32eceb8ba2080550aa0a9b5a0 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 2 Jun 2020 15:32:36 +1000 Subject: [PATCH 1/8] Issue #4919 - test graceful stop for jetty and javax ws containers Signed-off-by: Lachlan Roberts --- .../javax/tests/GracefulCloseTest.java | 127 ++++++++++++++++++ .../websocket/tests/GracefulCloseTest.java | 110 +++++++++++++++ 2 files changed, 237 insertions(+) create mode 100644 jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java create mode 100644 jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java diff --git a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java new file mode 100644 index 000000000000..bb9593db2a6d --- /dev/null +++ b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java @@ -0,0 +1,127 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.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.javax.tests; + +import java.net.URI; +import java.util.Objects; +import java.util.concurrent.TimeUnit; +import javax.websocket.CloseReason; +import javax.websocket.EndpointConfig; +import javax.websocket.Session; +import javax.websocket.server.ServerEndpoint; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.util.BlockingArrayQueue; +import org.eclipse.jetty.websocket.javax.client.internal.JavaxWebSocketClientContainer; +import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer; +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class GracefulCloseTest +{ + private static final BlockingArrayQueue serverEndpoints = new BlockingArrayQueue<>(); + private Server server; + private URI serverUri; + private JavaxWebSocketClientContainer client; + + @BeforeEach + public void before() throws Exception + { + server = new Server(); + ServerConnector connector = new ServerConnector(server); + server.addConnector(connector); + + ServletContextHandler contextHandler = new ServletContextHandler(); + contextHandler.setContextPath("/"); + server.setHandler(contextHandler); + JavaxWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> + container.addEndpoint(ServerSocket.class)); + server.start(); + serverUri = WSURI.toWebsocket(server.getURI()); + + client = new JavaxWebSocketClientContainer(); + client.start(); + } + + @AfterEach + public void after() throws Exception + { + client.stop(); + server.stop(); + } + + @ServerEndpoint("/") + public static class ServerSocket extends EventSocket.EchoSocket + { + @Override + public void onOpen(Session session, EndpointConfig endpointConfig) + { + serverEndpoints.add(this); + super.onOpen(session, endpointConfig); + } + } + + @Test + public void testClientStop() throws Exception + { + EventSocket clientEndpoint = new EventSocket(); + client.connectToServer(clientEndpoint, serverUri); + EventSocket serverEndpoint = Objects.requireNonNull(serverEndpoints.poll(5, TimeUnit.SECONDS)); + + client.stop(); + + // Check that the client endpoint was closed with the correct status code and no error. + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeReason.getCloseCode(), is(CloseReason.CloseCodes.GOING_AWAY)); + assertNull(clientEndpoint.error); + + // Check that the server endpoint was closed with the correct status code and no error. + assertTrue(serverEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(serverEndpoint.closeReason.getCloseCode(), is(CloseReason.CloseCodes.GOING_AWAY)); + assertNull(serverEndpoint.error); + } + + @Test + public void testServerStop() throws Exception + { + EventSocket clientEndpoint = new EventSocket(); + client.connectToServer(clientEndpoint, serverUri); + EventSocket serverEndpoint = Objects.requireNonNull(serverEndpoints.poll(5, TimeUnit.SECONDS)); + + server.stop(); + + // Check that the client endpoint was closed with the correct status code and no error. + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeReason.getCloseCode(), is(CloseReason.CloseCodes.GOING_AWAY)); + assertNull(clientEndpoint.error); + + // Check that the server endpoint was closed with the correct status code and no error. + assertTrue(serverEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(serverEndpoint.closeReason.getCloseCode(), is(CloseReason.CloseCodes.GOING_AWAY)); + assertNull(serverEndpoint.error); + } +} diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java new file mode 100644 index 000000000000..326af686528f --- /dev/null +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java @@ -0,0 +1,110 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.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.tests; + +import java.net.URI; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.websocket.api.StatusCode; +import org.eclipse.jetty.websocket.api.util.WSURI; +import org.eclipse.jetty.websocket.client.WebSocketClient; +import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class GracefulCloseTest +{ + private final EventSocket serverEndpoint = new EchoSocket(); + private Server server; + private URI serverUri; + private WebSocketClient client; + + @BeforeEach + public void before() throws Exception + { + server = new Server(); + ServerConnector connector = new ServerConnector(server); + server.addConnector(connector); + + ServletContextHandler contextHandler = new ServletContextHandler(); + contextHandler.setContextPath("/"); + server.setHandler(contextHandler); + JettyWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> + container.addMapping("/", ((req, resp) -> serverEndpoint))); + server.start(); + serverUri = WSURI.toWebsocket(server.getURI()); + + client = new WebSocketClient(); + client.start(); + } + + @AfterEach + public void after() throws Exception + { + client.stop(); + server.stop(); + } + + @Test + public void testClientStop() throws Exception + { + EventSocket clientEndpoint = new EventSocket(); + client.connect(clientEndpoint, serverUri).get(5, TimeUnit.SECONDS); + + client.stop(); + + // Check that the client endpoint was closed with the correct status code and no error. + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeCode, is(StatusCode.SHUTDOWN)); + assertNull(clientEndpoint.error); + + // Check that the server endpoint was closed with the correct status code and no error. + assertTrue(serverEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(serverEndpoint.closeCode, is(StatusCode.SHUTDOWN)); + assertNull(serverEndpoint.error); + } + + @Test + public void testServerStop() throws Exception + { + EventSocket clientEndpoint = new EventSocket(); + client.connect(clientEndpoint, serverUri).get(5, TimeUnit.SECONDS); + + server.stop(); + + // Check that the client endpoint was closed with the correct status code and no error. + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeCode, is(StatusCode.SHUTDOWN)); + assertNull(clientEndpoint.error); + + // Check that the server endpoint was closed with the correct status code and no error. + assertTrue(serverEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(serverEndpoint.closeCode, is(StatusCode.SHUTDOWN)); + assertNull(serverEndpoint.error); + } +} From bebe6fd13876a3de55be8bd71c4fc4b6e1369412 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 2 Jun 2020 15:37:55 +1000 Subject: [PATCH 2/8] Issue #4919 - always stop SessionTracker before closing connections Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/server/Server.java | 5 ++++- .../JavaxWebSocketClientContainer.java | 7 +++++++ .../javax/common/JavaxWebSocketContainer.java | 2 +- .../javax/common/SessionTracker.java | 2 +- .../JavaxWebSocketServerContainer.java | 19 +++++++++++++++++- .../websocket/javax/tests/EventSocket.java | 19 ++++++++++++++++++ .../websocket/client/WebSocketClient.java | 7 +++++++ .../websocket/common/SessionTracker.java | 2 +- .../server/JettyWebSocketServerContainer.java | 20 ++++++++++++++++++- 9 files changed, 77 insertions(+), 6 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 01cad1a17a60..b483b8ca2ed6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -27,6 +27,7 @@ import java.util.Enumeration; import java.util.List; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import javax.servlet.ServletContext; @@ -468,12 +469,14 @@ protected void doStop() throws Exception MultiException mex = new MultiException(); + // Initiate graceful shutdown but only wait for it if stopTimeout is set. + CompletableFuture shutdown = Graceful.shutdown(this); if (getStopTimeout() > 0) { long end = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(getStopTimeout()); try { - Graceful.shutdown(this).get(getStopTimeout(), TimeUnit.MILLISECONDS); + shutdown.get(getStopTimeout(), TimeUnit.MILLISECONDS); } catch (Throwable e) { diff --git a/jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java b/jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java index 8202b43a44a6..dcce8ecfb3cc 100644 --- a/jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java +++ b/jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java @@ -276,4 +276,11 @@ private ClientEndpointConfig getAnnotatedConfig(Object endpoint) throws Deployme return new AnnotatedClientEndpointConfig(anno); } + + @Override + protected void doStop() throws Exception + { + sessionTracker.stop(); + super.doStop(); + } } diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java index 44c07a2815bb..57a7548e078e 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java @@ -40,8 +40,8 @@ public abstract class JavaxWebSocketContainer extends ContainerLifeCycle implements javax.websocket.WebSocketContainer { private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketContainer.class); - private final SessionTracker sessionTracker = new SessionTracker(); private final List sessionListeners = new ArrayList<>(); + protected final SessionTracker sessionTracker = new SessionTracker(); protected final Configuration.ConfigurationCustomizer defaultCustomizer = new Configuration.ConfigurationCustomizer(); protected final WebSocketComponents components; diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java index 6010f9bae72e..600540259a34 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java @@ -33,7 +33,7 @@ public class SessionTracker extends AbstractLifeCycle implements JavaxWebSocketS { private static final Logger LOG = LoggerFactory.getLogger(SessionTracker.class); - private CopyOnWriteArraySet sessions = new CopyOnWriteArraySet<>(); + private final CopyOnWriteArraySet sessions = new CopyOnWriteArraySet<>(); public Set getSessions() { diff --git a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java index 59f211b5f2e5..642ebf0cc979 100644 --- a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java +++ b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.function.Function; import javax.servlet.ServletContext; @@ -33,6 +34,7 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.util.annotation.ManagedObject; +import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient; @@ -45,7 +47,7 @@ import org.slf4j.LoggerFactory; @ManagedObject("JSR356 Server Container") -public class JavaxWebSocketServerContainer extends JavaxWebSocketClientContainer implements javax.websocket.server.ServerContainer, LifeCycle.Listener +public class JavaxWebSocketServerContainer extends JavaxWebSocketClientContainer implements javax.websocket.server.ServerContainer, LifeCycle.Listener, Graceful { public static final String JAVAX_WEBSOCKET_CONTAINER_ATTRIBUTE = javax.websocket.server.ServerContainer.class.getName(); private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketServerContainer.class); @@ -260,4 +262,19 @@ protected void doStart() throws Exception deferredEndpointConfigs.clear(); } } + + @Override + public CompletableFuture shutdown() + { + LifeCycle.stop(sessionTracker); + CompletableFuture shutdown = new CompletableFuture<>(); + shutdown.complete(null); + return shutdown; + } + + @Override + public boolean isShutdown() + { + return sessionTracker.isStopped(); + } } diff --git a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java index a69db90ba456..a8b7d1a5fe92 100644 --- a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java +++ b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java @@ -98,4 +98,23 @@ public void onError(Throwable cause) error = cause; errorLatch.countDown(); } + + @ServerEndpoint("/") + @ClientEndpoint + public static class EchoSocket extends EventSocket + { + @Override + public void onMessage(String message) throws IOException + { + super.onMessage(message); + session.getBasicRemote().sendText(message); + } + + @Override + public void onMessage(ByteBuffer message) throws IOException + { + super.onMessage(message); + session.getBasicRemote().sendBinary(message); + } + } } diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index 56f74cc45299..9781637206e7 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java @@ -386,6 +386,13 @@ public boolean isStopAtShutdown() return stopAtShutdown; } + @Override + protected void doStop() throws Exception + { + sessionTracker.stop(); + super.doStop(); + } + @Override public String toString() { diff --git a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java index e963800ae9f9..9c504802e99d 100644 --- a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java +++ b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java @@ -29,7 +29,7 @@ public class SessionTracker extends AbstractLifeCycle implements WebSocketSessionListener { - private List sessions = new CopyOnWriteArrayList<>(); + private final List sessions = new CopyOnWriteArrayList<>(); public Collection getSessions() { diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java index bc6c85030e86..77d84ff0703b 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.function.Consumer; import javax.servlet.ServletContext; @@ -29,6 +30,7 @@ import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.WebSocketBehavior; @@ -47,7 +49,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class JettyWebSocketServerContainer extends ContainerLifeCycle implements WebSocketContainer, WebSocketPolicy, LifeCycle.Listener +public class JettyWebSocketServerContainer extends ContainerLifeCycle implements WebSocketContainer, WebSocketPolicy, LifeCycle.Listener, Graceful { public static final String JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE = WebSocketContainer.class.getName(); @@ -118,6 +120,7 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl frameHandlerFactory = factory; addSessionListener(sessionTracker); + addBean(sessionTracker); } public void addMapping(String pathSpec, JettyWebSocketCreator creator) @@ -260,4 +263,19 @@ public void setAutoFragment(boolean autoFragment) { customizer.setAutoFragment(autoFragment); } + + @Override + public CompletableFuture shutdown() + { + LifeCycle.stop(sessionTracker); + CompletableFuture shutdown = new CompletableFuture<>(); + shutdown.complete(null); + return shutdown; + } + + @Override + public boolean isShutdown() + { + return sessionTracker.isStopped(); + } } From 9423a8753e49bc1dcc61121ab18286f855c23b3a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 2 Jun 2020 16:10:48 +1000 Subject: [PATCH 3/8] fix some websocket jpms errors Signed-off-by: Lachlan Roberts --- .../websocket-core-common/src/main/java/module-info.java | 3 ++- jetty-websocket/websocket-util/src/main/java/module-info.java | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/jetty-websocket/websocket-core-common/src/main/java/module-info.java b/jetty-websocket/websocket-core-common/src/main/java/module-info.java index c2fb2c5276c9..4ca42a93b258 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/module-info.java +++ b/jetty-websocket/websocket-core-common/src/main/java/module-info.java @@ -29,8 +29,9 @@ exports org.eclipse.jetty.websocket.core.internal to org.eclipse.jetty.websocket.core.client, org.eclipse.jetty.websocket.core.server, org.eclipse.jetty.util; requires org.eclipse.jetty.http; - requires org.eclipse.jetty.io; requires org.slf4j; + requires transitive org.eclipse.jetty.io; + requires transitive org.eclipse.jetty.util; uses Extension; diff --git a/jetty-websocket/websocket-util/src/main/java/module-info.java b/jetty-websocket/websocket-util/src/main/java/module-info.java index 22dafa0caea3..17b2c628bbd3 100644 --- a/jetty-websocket/websocket-util/src/main/java/module-info.java +++ b/jetty-websocket/websocket-util/src/main/java/module-info.java @@ -21,8 +21,8 @@ exports org.eclipse.jetty.websocket.util; exports org.eclipse.jetty.websocket.util.messages; - requires org.eclipse.jetty.util; requires org.slf4j; - requires org.eclipse.jetty.io; + requires transitive org.eclipse.jetty.util; + requires transitive org.eclipse.jetty.io; requires transitive org.eclipse.jetty.websocket.core.common; } From 0818f54be8b33e541843f8b5aa44550e617bf35f Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 20 Jul 2020 15:32:46 +1000 Subject: [PATCH 4/8] cleanup the ws javax EchoSocket test class Signed-off-by: Lachlan Roberts --- .../websocket/javax/tests/EchoSocket.java | 43 +++++++++++++++++++ .../websocket/javax/tests/EventSocket.java | 19 -------- .../javax/tests/GracefulCloseTest.java | 2 +- .../tests/coders/EncoderLifeCycleTest.java | 2 +- 4 files changed, 45 insertions(+), 21 deletions(-) create mode 100644 jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EchoSocket.java diff --git a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EchoSocket.java b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EchoSocket.java new file mode 100644 index 000000000000..c18d8b665a43 --- /dev/null +++ b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EchoSocket.java @@ -0,0 +1,43 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.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.javax.tests; + +import java.io.IOException; +import java.nio.ByteBuffer; +import javax.websocket.ClientEndpoint; +import javax.websocket.server.ServerEndpoint; + +@ServerEndpoint("/") +@ClientEndpoint +public class EchoSocket extends EventSocket +{ + @Override + public void onMessage(String message) throws IOException + { + super.onMessage(message); + session.getBasicRemote().sendText(message); + } + + @Override + public void onMessage(ByteBuffer message) throws IOException + { + super.onMessage(message); + session.getBasicRemote().sendBinary(message); + } +} diff --git a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java index a8b7d1a5fe92..a69db90ba456 100644 --- a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java +++ b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/EventSocket.java @@ -98,23 +98,4 @@ public void onError(Throwable cause) error = cause; errorLatch.countDown(); } - - @ServerEndpoint("/") - @ClientEndpoint - public static class EchoSocket extends EventSocket - { - @Override - public void onMessage(String message) throws IOException - { - super.onMessage(message); - session.getBasicRemote().sendText(message); - } - - @Override - public void onMessage(ByteBuffer message) throws IOException - { - super.onMessage(message); - session.getBasicRemote().sendBinary(message); - } - } } diff --git a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java index bb9593db2a6d..890065cb2831 100644 --- a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java +++ b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java @@ -75,7 +75,7 @@ public void after() throws Exception } @ServerEndpoint("/") - public static class ServerSocket extends EventSocket.EchoSocket + public static class ServerSocket extends EchoSocket { @Override public void onOpen(Session session, EndpointConfig endpointConfig) diff --git a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/coders/EncoderLifeCycleTest.java b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/coders/EncoderLifeCycleTest.java index cc03423accdc..88657bbbd743 100644 --- a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/coders/EncoderLifeCycleTest.java +++ b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/coders/EncoderLifeCycleTest.java @@ -40,7 +40,7 @@ import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketSession; import org.eclipse.jetty.websocket.javax.common.encoders.AvailableEncoders; import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer; -import org.eclipse.jetty.websocket.javax.tests.EventSocket.EchoSocket; +import org.eclipse.jetty.websocket.javax.tests.EchoSocket; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.params.ParameterizedTest; From 9f7f2e3e56366b95b46ed2e861dd9a3a9325c3d4 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 20 Jul 2020 15:49:15 +1000 Subject: [PATCH 5/8] WebSocket server now only closes gracefully if the Server stopTimeout is set Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/server/Server.java | 5 +---- .../internal/JavaxWebSocketServerContainer.java | 17 +++++++++++++++-- .../javax/tests/GracefulCloseTest.java | 3 +++ .../server/JettyWebSocketServerContainer.java | 17 +++++++++++++++-- .../websocket/tests/GracefulCloseTest.java | 3 +++ 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 5eb484b328dc..8cd868c9045b 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -27,7 +27,6 @@ import java.util.Enumeration; import java.util.List; import java.util.Set; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import javax.servlet.ServletContext; @@ -469,14 +468,12 @@ protected void doStop() throws Exception MultiException mex = new MultiException(); - // Initiate graceful shutdown but only wait for it if stopTimeout is set. - CompletableFuture shutdown = Graceful.shutdown(this); if (getStopTimeout() > 0) { long end = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(getStopTimeout()); try { - shutdown.get(getStopTimeout(), TimeUnit.MILLISECONDS); + Graceful.shutdown(this).get(getStopTimeout(), TimeUnit.MILLISECONDS); } catch (Throwable e) { diff --git a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java index 9786a3fec92a..bbe894498966 100644 --- a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java +++ b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java @@ -307,9 +307,22 @@ protected void doStart() throws Exception @Override public CompletableFuture shutdown() { - LifeCycle.stop(sessionTracker); CompletableFuture shutdown = new CompletableFuture<>(); - shutdown.complete(null); + new Thread(() -> + { + try + { + LifeCycle.stop(sessionTracker); + } + catch (Throwable t) + { + LOG.warn("Error while stopping SessionTracker", t); + } + finally + { + shutdown.complete(null); + } + }).start(); return shutdown; } diff --git a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java index 890065cb2831..6a821a2ae928 100644 --- a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java +++ b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java @@ -63,6 +63,9 @@ public void before() throws Exception server.start(); serverUri = WSURI.toWebsocket(server.getURI()); + // StopTimeout is necessary for the websocket server sessions to gracefully close. + server.setStopTimeout(1000); + client = new JavaxWebSocketClientContainer(); client.start(); } diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java index 77d84ff0703b..91083607d8b6 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java @@ -267,9 +267,22 @@ public void setAutoFragment(boolean autoFragment) @Override public CompletableFuture shutdown() { - LifeCycle.stop(sessionTracker); CompletableFuture shutdown = new CompletableFuture<>(); - shutdown.complete(null); + new Thread(() -> + { + try + { + LifeCycle.stop(sessionTracker); + } + catch (Throwable t) + { + LOG.warn("Error while stopping SessionTracker", t); + } + finally + { + shutdown.complete(null); + } + }).start(); return shutdown; } diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java index 326af686528f..02838c67dd0c 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java @@ -59,6 +59,9 @@ public void before() throws Exception server.start(); serverUri = WSURI.toWebsocket(server.getURI()); + // StopTimeout is necessary for the websocket server sessions to gracefully close. + server.setStopTimeout(1000); + client = new WebSocketClient(); client.start(); } From 695d239ac56721338bae051e88360d437d0968f5 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 23 Jul 2020 00:09:43 +1000 Subject: [PATCH 6/8] Issue #4919 - all websocket containers to implement Graceful shutdown interface Signed-off-by: Lachlan Roberts --- .../JavaxWebSocketClientContainer.java | 7 -- .../javax/common/JavaxWebSocketContainer.java | 25 ++++++- .../JavaxWebSocketServerContainer.java | 32 +------- .../javax/tests/GracefulCloseTest.java | 3 + .../websocket/client/WebSocketClient.java | 39 +++++++++- .../server/JettyWebSocketServerContainer.java | 27 +++---- .../websocket/tests/GracefulCloseTest.java | 1 + .../jetty/websocket/util/ShutdownUtil.java | 75 +++++++++++++++++++ 8 files changed, 148 insertions(+), 61 deletions(-) create mode 100644 jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java diff --git a/jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java b/jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java index dcce8ecfb3cc..8202b43a44a6 100644 --- a/jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java +++ b/jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java @@ -276,11 +276,4 @@ private ClientEndpointConfig getAnnotatedConfig(Object endpoint) throws Deployme return new AnnotatedClientEndpointConfig(anno); } - - @Override - protected void doStop() throws Exception - { - sessionTracker.stop(); - super.doStop(); - } } diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java index 57a7548e078e..5c4f5380afbe 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java @@ -23,6 +23,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.function.Consumer; import javax.websocket.Extension; @@ -31,13 +32,15 @@ import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.DecoratedObjectFactory; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.websocket.core.Configuration; import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.WebSocketExtensionRegistry; +import org.eclipse.jetty.websocket.util.ShutdownUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public abstract class JavaxWebSocketContainer extends ContainerLifeCycle implements javax.websocket.WebSocketContainer +public abstract class JavaxWebSocketContainer extends ContainerLifeCycle implements javax.websocket.WebSocketContainer, Graceful { private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketContainer.class); private final List sessionListeners = new ArrayList<>(); @@ -49,7 +52,6 @@ public JavaxWebSocketContainer(WebSocketComponents components) { this.components = components; addSessionListener(sessionTracker); - addBean(sessionTracker); } public abstract Executor getExecutor(); @@ -198,4 +200,23 @@ public void notifySessionListeners(Consumer consu } } } + + @Override + protected void doStart() throws Exception + { + sessionTracker.start(); + super.doStart(); + } + + @Override + public CompletableFuture shutdown() + { + return ShutdownUtil.shutdown(sessionTracker); + } + + @Override + public boolean isShutdown() + { + return sessionTracker.isStopped(); + } } diff --git a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java index bbe894498966..f3561078a6ec 100644 --- a/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java +++ b/jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java @@ -21,7 +21,6 @@ import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.function.Function; import javax.servlet.ServletContext; @@ -35,7 +34,6 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.util.annotation.ManagedObject; -import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient; @@ -50,7 +48,7 @@ import org.slf4j.LoggerFactory; @ManagedObject("JSR356 Server Container") -public class JavaxWebSocketServerContainer extends JavaxWebSocketClientContainer implements javax.websocket.server.ServerContainer, LifeCycle.Listener, Graceful +public class JavaxWebSocketServerContainer extends JavaxWebSocketClientContainer implements javax.websocket.server.ServerContainer, LifeCycle.Listener { public static final String JAVAX_WEBSOCKET_CONTAINER_ATTRIBUTE = javax.websocket.server.ServerContainer.class.getName(); private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketServerContainer.class); @@ -303,32 +301,4 @@ protected void doStart() throws Exception deferredEndpointConfigs.clear(); } } - - @Override - public CompletableFuture shutdown() - { - CompletableFuture shutdown = new CompletableFuture<>(); - new Thread(() -> - { - try - { - LifeCycle.stop(sessionTracker); - } - catch (Throwable t) - { - LOG.warn("Error while stopping SessionTracker", t); - } - finally - { - shutdown.complete(null); - } - }).start(); - return shutdown; - } - - @Override - public boolean isShutdown() - { - return sessionTracker.isStopped(); - } } diff --git a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java index 6a821a2ae928..b27453454840 100644 --- a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java +++ b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/GracefulCloseTest.java @@ -30,6 +30,7 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.util.BlockingArrayQueue; +import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.websocket.javax.client.internal.JavaxWebSocketClientContainer; import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer; import org.junit.jupiter.api.AfterEach; @@ -95,6 +96,8 @@ public void testClientStop() throws Exception client.connectToServer(clientEndpoint, serverUri); EventSocket serverEndpoint = Objects.requireNonNull(serverEndpoints.poll(5, TimeUnit.SECONDS)); + // There is no API for a Javax WebSocketContainer stop timeout. + Graceful.shutdown(client).get(5, TimeUnit.SECONDS); client.stop(); // Check that the client endpoint was closed with the correct status code and no error. diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index 399aedab381e..4519862aaf90 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java @@ -29,6 +29,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import org.eclipse.jetty.client.HttpClient; @@ -38,6 +39,7 @@ import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.util.DecoratedObjectFactory; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.ShutdownThread; import org.eclipse.jetty.websocket.api.Session; @@ -53,10 +55,11 @@ import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.client.UpgradeListener; import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient; +import org.eclipse.jetty.websocket.util.ShutdownUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class WebSocketClient extends ContainerLifeCycle implements WebSocketPolicy, WebSocketContainer +public class WebSocketClient extends ContainerLifeCycle implements WebSocketPolicy, WebSocketContainer, Graceful { private static final Logger LOG = LoggerFactory.getLogger(WebSocketClient.class); private final WebSocketCoreClient coreClient; @@ -67,6 +70,7 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketPoli private final Configuration.ConfigurationCustomizer configurationCustomizer = new Configuration.ConfigurationCustomizer(); private final WebSocketComponents components = new WebSocketComponents(); private boolean stopAtShutdown = false; + private long _stopTimeout = 200; /** * Instantiate a WebSocketClient with defaults @@ -91,7 +95,6 @@ public WebSocketClient(HttpClient httpClient) frameHandlerFactory = new JettyWebSocketFrameHandlerFactory(this); sessionListeners.add(sessionTracker); - addBean(sessionTracker); } public CompletableFuture connect(Object websocket, URI toUri) throws IOException @@ -380,18 +383,48 @@ public synchronized void setStopAtShutdown(boolean stop) stopAtShutdown = stop; } + public void setStopTimeout(long stopTimeout) + { + _stopTimeout = stopTimeout; + } + + public long getStopTimeout() + { + return _stopTimeout; + } + public boolean isStopAtShutdown() { return stopAtShutdown; } + @Override + protected void doStart() throws Exception + { + sessionTracker.start(); + super.doStart(); + } + @Override protected void doStop() throws Exception { - sessionTracker.stop(); + if (getStopTimeout() > 0) + Graceful.shutdown(this).get(getStopTimeout(), TimeUnit.MILLISECONDS); super.doStop(); } + @Override + public CompletableFuture shutdown() + { + return ShutdownUtil.shutdown(sessionTracker); + } + + @Override + public boolean isShutdown() + { + return sessionTracker.isStopped(); + } + @Override public String toString() { diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java index 91083607d8b6..2bf701dfd68f 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java @@ -44,6 +44,7 @@ import org.eclipse.jetty.websocket.core.server.WebSocketServerComponents; import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; import org.eclipse.jetty.websocket.server.internal.JettyServerFrameHandlerFactory; +import org.eclipse.jetty.websocket.util.ShutdownUtil; import org.eclipse.jetty.websocket.util.server.internal.FrameHandlerFactory; import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping; import org.slf4j.Logger; @@ -120,7 +121,6 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl frameHandlerFactory = factory; addSessionListener(sessionTracker); - addBean(sessionTracker); } public void addMapping(String pathSpec, JettyWebSocketCreator creator) @@ -264,26 +264,17 @@ public void setAutoFragment(boolean autoFragment) customizer.setAutoFragment(autoFragment); } + @Override + protected void doStart() throws Exception + { + sessionTracker.start(); + super.doStart(); + } + @Override public CompletableFuture shutdown() { - CompletableFuture shutdown = new CompletableFuture<>(); - new Thread(() -> - { - try - { - LifeCycle.stop(sessionTracker); - } - catch (Throwable t) - { - LOG.warn("Error while stopping SessionTracker", t); - } - finally - { - shutdown.complete(null); - } - }).start(); - return shutdown; + return ShutdownUtil.shutdown(sessionTracker); } @Override diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java index 02838c67dd0c..d3157d0f2836 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/GracefulCloseTest.java @@ -63,6 +63,7 @@ public void before() throws Exception server.setStopTimeout(1000); client = new WebSocketClient(); + client.setStopTimeout(1000); client.start(); } diff --git a/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java b/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java new file mode 100644 index 000000000000..739fdac249ea --- /dev/null +++ b/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java @@ -0,0 +1,75 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 +// +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.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.util; + +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicReference; + +import org.eclipse.jetty.util.component.LifeCycle; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class ShutdownUtil +{ + private static final Logger LOG = LoggerFactory.getLogger(ShutdownUtil.class); + + /** + * Shutdown a {@link LifeCycle} in a new daemon thread and be notified on the result in a {@link CompletableFuture}. + * @param lifeCycle the LifeCycle to stop. + * @return the CompletableFuture to be notified when the stop either completes or fails. + */ + public static CompletableFuture shutdown(LifeCycle lifeCycle) + { + AtomicReference stopThreadReference = new AtomicReference<>(); + CompletableFuture shutdown = new CompletableFuture<>() + { + @Override + public boolean cancel(boolean mayInterruptIfRunning) + { + boolean canceled = super.cancel(mayInterruptIfRunning); + if (canceled && mayInterruptIfRunning) + { + Thread thread = stopThreadReference.get(); + if (thread != null) + thread.interrupt(); + } + + return canceled; + } + }; + + Thread stopThread = new Thread(() -> + { + try + { + lifeCycle.stop(); + shutdown.complete(null); + } + catch (Throwable t) + { + LOG.warn("Error while stopping {}", lifeCycle, t); + shutdown.completeExceptionally(t); + } + }); + stopThread.setDaemon(true); + stopThreadReference.set(stopThread); + stopThread.start(); + return shutdown; + } +} From 9e383f0891fdf838636c0193f954bdb54539d602 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 28 Jul 2020 16:48:49 +1000 Subject: [PATCH 7/8] Issue #4919 - changes from review Signed-off-by: Lachlan Roberts --- .../websocket/javax/common/JavaxWebSocketContainer.java | 2 +- .../jetty/websocket/javax/common/SessionTracker.java | 3 +++ .../eclipse/jetty/websocket/client/WebSocketClient.java | 8 ++++++-- .../eclipse/jetty/websocket/common/SessionTracker.java | 3 +++ .../websocket/server/JettyWebSocketServerContainer.java | 2 +- .../org/eclipse/jetty/websocket/util/ShutdownUtil.java | 4 ++-- 6 files changed, 16 insertions(+), 6 deletions(-) diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java index 5c4f5380afbe..527f06cbe049 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java @@ -211,7 +211,7 @@ protected void doStart() throws Exception @Override public CompletableFuture shutdown() { - return ShutdownUtil.shutdown(sessionTracker); + return ShutdownUtil.stop(sessionTracker); } @Override diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java index 600540259a34..0663c08b84fd 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java @@ -57,6 +57,9 @@ protected void doStop() throws Exception { for (Session session : sessions) { + if (Thread.interrupted()) + break; + try { // GOING_AWAY is abnormal close status so it will hard close connection after sent. diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index 48ccb97ab00a..e1bdedc37bb1 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java @@ -71,7 +71,7 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketPoli private final Configuration.ConfigurationCustomizer configurationCustomizer = new Configuration.ConfigurationCustomizer(); private final WebSocketComponents components = new WebSocketComponents(); private boolean stopAtShutdown = false; - private long _stopTimeout = 200; + private long _stopTimeout = Long.MAX_VALUE; /** * Instantiate a WebSocketClient with defaults @@ -391,6 +391,10 @@ public synchronized void setStopAtShutdown(boolean stop) stopAtShutdown = stop; } + /** + * The timeout to allow all remaining open Sessions to be closed gracefully using the close code {@link org.eclipse.jetty.websocket.api.StatusCode#SHUTDOWN}. + * @param stopTimeout the time in ms to wait for the graceful close, use a value less than or equal to 0 to not gracefully close. + */ public void setStopTimeout(long stopTimeout) { _stopTimeout = stopTimeout; @@ -424,7 +428,7 @@ protected void doStop() throws Exception @Override public CompletableFuture shutdown() { - return ShutdownUtil.shutdown(sessionTracker); + return ShutdownUtil.stop(sessionTracker); } @Override diff --git a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java index 9c504802e99d..a2cb83a8a016 100644 --- a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java +++ b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java @@ -53,6 +53,9 @@ protected void doStop() throws Exception { for (Session session : sessions) { + if (Thread.interrupted()) + break; + // SHUTDOWN is abnormal close status so it will hard close connection after sent. session.close(StatusCode.SHUTDOWN, "Container being shut down"); } diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java index 7fa99c2465c5..708d02904ff2 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java @@ -293,7 +293,7 @@ protected void doStart() throws Exception @Override public CompletableFuture shutdown() { - return ShutdownUtil.shutdown(sessionTracker); + return ShutdownUtil.stop(sessionTracker); } @Override diff --git a/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java b/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java index 739fdac249ea..cded9dce6aa9 100644 --- a/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java +++ b/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java @@ -30,11 +30,11 @@ public class ShutdownUtil private static final Logger LOG = LoggerFactory.getLogger(ShutdownUtil.class); /** - * Shutdown a {@link LifeCycle} in a new daemon thread and be notified on the result in a {@link CompletableFuture}. + * Stop a {@link LifeCycle} in a new daemon thread and be notified of the result in a {@link CompletableFuture}. * @param lifeCycle the LifeCycle to stop. * @return the CompletableFuture to be notified when the stop either completes or fails. */ - public static CompletableFuture shutdown(LifeCycle lifeCycle) + public static CompletableFuture stop(LifeCycle lifeCycle) { AtomicReference stopThreadReference = new AtomicReference<>(); CompletableFuture shutdown = new CompletableFuture<>() From 1be02209883dd5b5196742a76508d179b205dee5 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 28 Jul 2020 17:38:41 +1000 Subject: [PATCH 8/8] Issue #4919 - make the SessionTracker the one to implement Graceful shutdown Signed-off-by: Lachlan Roberts --- .../jetty/util/component/Graceful.java | 50 +++++++++++++ .../javax/common/JavaxWebSocketContainer.java | 25 +------ .../javax/common/SessionTracker.java | 47 +++++++----- .../websocket/client/WebSocketClient.java | 23 +----- .../websocket/common/SessionTracker.java | 41 ++++++++-- .../server/JettyWebSocketServerContainer.java | 25 +------ .../jetty/websocket/util/ShutdownUtil.java | 75 ------------------- 7 files changed, 119 insertions(+), 167 deletions(-) delete mode 100644 jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java index 164a702c99d9..b5e54c60eab4 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/Graceful.java @@ -139,4 +139,54 @@ static CompletableFuture shutdown(Container component) return CompletableFuture.allOf(gracefuls.stream().map(Graceful::shutdown).toArray(CompletableFuture[]::new)); } + + /** + * Utility method to execute a {@link ThrowingRunnable} in a new daemon thread and + * be notified of the result in a {@link CompletableFuture}. + * @param runnable the ThrowingRunnable to run. + * @return the CompletableFuture to be notified when the runnable either completes or fails. + */ + static CompletableFuture shutdown(ThrowingRunnable runnable) + { + AtomicReference stopThreadReference = new AtomicReference<>(); + CompletableFuture shutdown = new CompletableFuture<>() + { + @Override + public boolean cancel(boolean mayInterruptIfRunning) + { + boolean canceled = super.cancel(mayInterruptIfRunning); + if (canceled && mayInterruptIfRunning) + { + Thread thread = stopThreadReference.get(); + if (thread != null) + thread.interrupt(); + } + + return canceled; + } + }; + + Thread stopThread = new Thread(() -> + { + try + { + runnable.run(); + shutdown.complete(null); + } + catch (Throwable t) + { + shutdown.completeExceptionally(t); + } + }); + stopThread.setDaemon(true); + stopThreadReference.set(stopThread); + stopThread.start(); + return shutdown; + } + + @FunctionalInterface + interface ThrowingRunnable + { + void run() throws Exception; + } } diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java index 527f06cbe049..57a7548e078e 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketContainer.java @@ -23,7 +23,6 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.function.Consumer; import javax.websocket.Extension; @@ -32,15 +31,13 @@ import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.DecoratedObjectFactory; import org.eclipse.jetty.util.component.ContainerLifeCycle; -import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.websocket.core.Configuration; import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.WebSocketExtensionRegistry; -import org.eclipse.jetty.websocket.util.ShutdownUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public abstract class JavaxWebSocketContainer extends ContainerLifeCycle implements javax.websocket.WebSocketContainer, Graceful +public abstract class JavaxWebSocketContainer extends ContainerLifeCycle implements javax.websocket.WebSocketContainer { private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketContainer.class); private final List sessionListeners = new ArrayList<>(); @@ -52,6 +49,7 @@ public JavaxWebSocketContainer(WebSocketComponents components) { this.components = components; addSessionListener(sessionTracker); + addBean(sessionTracker); } public abstract Executor getExecutor(); @@ -200,23 +198,4 @@ public void notifySessionListeners(Consumer consu } } } - - @Override - protected void doStart() throws Exception - { - sessionTracker.start(); - super.doStart(); - } - - @Override - public CompletableFuture shutdown() - { - return ShutdownUtil.stop(sessionTracker); - } - - @Override - public boolean isShutdown() - { - return sessionTracker.isStopped(); - } } diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java index 0663c08b84fd..74fd8e8b08d7 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/SessionTracker.java @@ -18,22 +18,20 @@ package org.eclipse.jetty.websocket.javax.common; -import java.io.IOException; import java.util.Collections; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArraySet; import javax.websocket.CloseReason; import javax.websocket.Session; import org.eclipse.jetty.util.component.AbstractLifeCycle; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.eclipse.jetty.util.component.Graceful; -public class SessionTracker extends AbstractLifeCycle implements JavaxWebSocketSessionListener +public class SessionTracker extends AbstractLifeCycle implements JavaxWebSocketSessionListener, Graceful { - private static final Logger LOG = LoggerFactory.getLogger(SessionTracker.class); - private final CopyOnWriteArraySet sessions = new CopyOnWriteArraySet<>(); + private boolean isShutdown = false; public Set getSessions() { @@ -52,25 +50,40 @@ public void onJavaxWebSocketSessionClosed(JavaxWebSocketSession session) sessions.remove(session); } + @Override + protected void doStart() throws Exception + { + isShutdown = false; + super.doStart(); + } + @Override protected void doStop() throws Exception { - for (Session session : sessions) - { - if (Thread.interrupted()) - break; + sessions.clear(); + super.doStop(); + } - try + @Override + public CompletableFuture shutdown() + { + isShutdown = true; + return Graceful.shutdown(() -> + { + for (Session session : sessions) { + if (Thread.interrupted()) + break; + // GOING_AWAY is abnormal close status so it will hard close connection after sent. session.close(new CloseReason(CloseReason.CloseCodes.GOING_AWAY, "Container being shut down")); } - catch (IOException e) - { - LOG.trace("IGNORED", e); - } - } + }); + } - super.doStop(); + @Override + public boolean isShutdown() + { + return isShutdown; } } diff --git a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index e1bdedc37bb1..091a7f7d6941 100644 --- a/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/jetty-websocket/websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java @@ -56,11 +56,10 @@ import org.eclipse.jetty.websocket.core.WebSocketComponents; import org.eclipse.jetty.websocket.core.client.UpgradeListener; import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient; -import org.eclipse.jetty.websocket.util.ShutdownUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class WebSocketClient extends ContainerLifeCycle implements WebSocketPolicy, WebSocketContainer, Graceful +public class WebSocketClient extends ContainerLifeCycle implements WebSocketPolicy, WebSocketContainer { private static final Logger LOG = LoggerFactory.getLogger(WebSocketClient.class); private final WebSocketCoreClient coreClient; @@ -96,6 +95,7 @@ public WebSocketClient(HttpClient httpClient) frameHandlerFactory = new JettyWebSocketFrameHandlerFactory(this); sessionListeners.add(sessionTracker); + addBean(sessionTracker); } public CompletableFuture connect(Object websocket, URI toUri) throws IOException @@ -410,13 +410,6 @@ public boolean isStopAtShutdown() return stopAtShutdown; } - @Override - protected void doStart() throws Exception - { - sessionTracker.start(); - super.doStart(); - } - @Override protected void doStop() throws Exception { @@ -425,18 +418,6 @@ protected void doStop() throws Exception super.doStop(); } - @Override - public CompletableFuture shutdown() - { - return ShutdownUtil.stop(sessionTracker); - } - - @Override - public boolean isShutdown() - { - return sessionTracker.isStopped(); - } - @Override public String toString() { diff --git a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java index a2cb83a8a016..1755bce59001 100644 --- a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java +++ b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/SessionTracker.java @@ -20,16 +20,19 @@ import java.util.Collection; import java.util.List; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.StatusCode; import org.eclipse.jetty.websocket.api.WebSocketSessionListener; -public class SessionTracker extends AbstractLifeCycle implements WebSocketSessionListener +public class SessionTracker extends AbstractLifeCycle implements WebSocketSessionListener, Graceful { private final List sessions = new CopyOnWriteArrayList<>(); + private boolean isShutdown = false; public Collection getSessions() { @@ -48,18 +51,40 @@ public void onWebSocketSessionClosed(Session session) sessions.remove(session); } + @Override + protected void doStart() throws Exception + { + isShutdown = false; + super.doStart(); + } + @Override protected void doStop() throws Exception { - for (Session session : sessions) + sessions.clear(); + super.doStop(); + } + + @Override + public CompletableFuture shutdown() + { + isShutdown = true; + return Graceful.shutdown(() -> { - if (Thread.interrupted()) - break; + for (Session session : sessions) + { + if (Thread.interrupted()) + break; - // SHUTDOWN is abnormal close status so it will hard close connection after sent. - session.close(StatusCode.SHUTDOWN, "Container being shut down"); - } + // SHUTDOWN is abnormal close status so it will hard close connection after sent. + session.close(StatusCode.SHUTDOWN, "Container being shut down"); + } + }); + } - super.doStop(); + @Override + public boolean isShutdown() + { + return isShutdown; } } diff --git a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java index 708d02904ff2..5eadef61ef3b 100644 --- a/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-websocket/websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/JettyWebSocketServerContainer.java @@ -22,7 +22,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.function.Consumer; import javax.servlet.ServletContext; @@ -30,7 +29,6 @@ import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.util.component.ContainerLifeCycle; -import org.eclipse.jetty.util.component.Graceful; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.WebSocketBehavior; @@ -45,13 +43,12 @@ import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; import org.eclipse.jetty.websocket.server.internal.JettyServerFrameHandlerFactory; import org.eclipse.jetty.websocket.util.ReflectUtils; -import org.eclipse.jetty.websocket.util.ShutdownUtil; import org.eclipse.jetty.websocket.util.server.internal.FrameHandlerFactory; import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class JettyWebSocketServerContainer extends ContainerLifeCycle implements WebSocketContainer, WebSocketPolicy, LifeCycle.Listener, Graceful +public class JettyWebSocketServerContainer extends ContainerLifeCycle implements WebSocketContainer, WebSocketPolicy, LifeCycle.Listener { public static final String JETTY_WEBSOCKET_CONTAINER_ATTRIBUTE = WebSocketContainer.class.getName(); @@ -122,6 +119,7 @@ public static JettyWebSocketServerContainer ensureContainer(ServletContext servl frameHandlerFactory = factory; addSessionListener(sessionTracker); + addBean(sessionTracker); } public void addMapping(String pathSpec, JettyWebSocketCreator creator) @@ -282,23 +280,4 @@ public void setAutoFragment(boolean autoFragment) { customizer.setAutoFragment(autoFragment); } - - @Override - protected void doStart() throws Exception - { - sessionTracker.start(); - super.doStart(); - } - - @Override - public CompletableFuture shutdown() - { - return ShutdownUtil.stop(sessionTracker); - } - - @Override - public boolean isShutdown() - { - return sessionTracker.isStopped(); - } } diff --git a/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java b/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java deleted file mode 100644 index cded9dce6aa9..000000000000 --- a/jetty-websocket/websocket-util/src/main/java/org/eclipse/jetty/websocket/util/ShutdownUtil.java +++ /dev/null @@ -1,75 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under -// the terms of the Eclipse Public License 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0 -// -// This Source Code may also be made available under the following -// Secondary Licenses when the conditions for such availability set -// forth in the Eclipse Public License, v. 2.0 are satisfied: -// the Apache License v2.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.util; - -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.atomic.AtomicReference; - -import org.eclipse.jetty.util.component.LifeCycle; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -public class ShutdownUtil -{ - private static final Logger LOG = LoggerFactory.getLogger(ShutdownUtil.class); - - /** - * Stop a {@link LifeCycle} in a new daemon thread and be notified of the result in a {@link CompletableFuture}. - * @param lifeCycle the LifeCycle to stop. - * @return the CompletableFuture to be notified when the stop either completes or fails. - */ - public static CompletableFuture stop(LifeCycle lifeCycle) - { - AtomicReference stopThreadReference = new AtomicReference<>(); - CompletableFuture shutdown = new CompletableFuture<>() - { - @Override - public boolean cancel(boolean mayInterruptIfRunning) - { - boolean canceled = super.cancel(mayInterruptIfRunning); - if (canceled && mayInterruptIfRunning) - { - Thread thread = stopThreadReference.get(); - if (thread != null) - thread.interrupt(); - } - - return canceled; - } - }; - - Thread stopThread = new Thread(() -> - { - try - { - lifeCycle.stop(); - shutdown.complete(null); - } - catch (Throwable t) - { - LOG.warn("Error while stopping {}", lifeCycle, t); - shutdown.completeExceptionally(t); - } - }); - stopThread.setDaemon(true); - stopThreadReference.set(stopThread); - stopThread.start(); - return shutdown; - } -}