Issue #5832 - shutdown Javax WSClientContainer with webapp if possible.

jetty-websocket/websocket-javax-client/src/main/java/module-info.java

@@ -20,6 +20,7 @@
     exports org.eclipse.jetty.websocket.javax.client;
     exports org.eclipse.jetty.websocket.javax.client.internal to org.eclipse.jetty.websocket.javax.server;
 
+    requires org.slf4j;
     requires org.eclipse.jetty.client;
     requires org.eclipse.jetty.websocket.core.client;
     requires org.eclipse.jetty.websocket.javax.common;

jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainerProvider.java

@@ -17,7 +17,6 @@
 import javax.websocket.WebSocketContainer;
 
 import org.eclipse.jetty.util.component.LifeCycle;
-import org.eclipse.jetty.util.thread.ShutdownThread;
 import org.eclipse.jetty.websocket.javax.client.internal.JavaxWebSocketClientContainer;
 
 /**
@@ -59,22 +58,7 @@ protected WebSocketContainer getContainer()
         // TODO: do we want to provide a non-standard way to configure to always return the same clientContainer based on a config somewhere? (system.property?)
 
         JavaxWebSocketClientContainer clientContainer = new JavaxWebSocketClientContainer();
-
-        // Register as JVM runtime shutdown hook?
-        ShutdownThread.register(clientContainer);
-
-        if (!clientContainer.isStarted())
-        {
-            try
-            {
-                clientContainer.start();
-            }
-            catch (Exception e)
-            {
-                throw new RuntimeException("Unable to start Client Container", e);
-            }
-        }
-
+        LifeCycle.start(clientContainer);
         return clientContainer;
     }
 }

jetty-websocket/websocket-javax-client/src/main/java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java

@@ -15,6 +15,7 @@
 
 import java.io.IOException;
 import java.net.URI;
+import java.util.EventListener;
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
@@ -33,6 +34,8 @@
 
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.util.annotation.ManagedObject;
+import org.eclipse.jetty.util.component.LifeCycle;
+import org.eclipse.jetty.util.thread.ShutdownThread;
 import org.eclipse.jetty.websocket.core.WebSocketComponents;
 import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient;
 import org.eclipse.jetty.websocket.core.exception.InvalidWebSocketException;
@@ -43,6 +46,8 @@
 import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketExtensionConfig;
 import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandler;
 import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandlerFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Container for Client use of the javax.websocket API.
@@ -52,6 +57,8 @@
 @ManagedObject("JSR356 Client Container")
 public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer implements javax.websocket.WebSocketContainer
 {
+    private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketClientContainer.class);
+
     protected WebSocketCoreClient coreClient;
     protected Function<WebSocketComponents, WebSocketCoreClient> coreClientFactory;
     private final JavaxWebSocketClientFrameHandlerFactory frameHandlerFactory;
@@ -271,4 +278,68 @@ private ClientEndpointConfig getAnnotatedConfig(Object endpoint) throws Deployme
 
         return new AnnotatedClientEndpointConfig(anno);
     }
+
+    @Override
+    protected void doStart() throws Exception
+    {
+        doClientStart();
+        super.doStart();
+    }
+
+    protected void doClientStart()
+    {
+        try
+        {
+            Object context = getClass().getClassLoader()
+                .loadClass("org.eclipse.jetty.server.handler.ContextHandler")
+                .getMethod("getCurrentContext")
+                .invoke(null);
+
+            Object contextHandler = context.getClass()
+                .getMethod("getContextHandler")
+                .invoke(context);
+
+            contextHandler.getClass()
+                .getMethod("addManaged", LifeCycle.class)
+                .invoke(contextHandler, this);
+
+            AbstractLifeCycleListener shutdownListener = new AbstractLifeCycleListener()
+            {
+                @Override
+                public void lifeCycleStopping(LifeCycle event)
+                {
+                    try
+                    {
+                        contextHandler.getClass()
+                            .getMethod("removeBean", Object.class)
+                            .invoke(contextHandler, JavaxWebSocketClientContainer.this);
+                    }
+                    catch (Throwable t)
+                    {
+                        LOG.warn("could not remove client WebSocketContainer bean from {}", contextHandler);
+                    }
+                }
+            };
+
+            contextHandler.getClass()
+                .getMethod("addEventListener", EventListener.class)
+                .invoke(contextHandler, shutdownListener);
+        }
+        catch (Throwable ignored)
+        {
+            ShutdownThread.register(this);
+        }
+    }
+
+    @Override
+    protected void doStop() throws Exception
+    {
+        super.doStop();
+        doClientStop();
+    }
+
+    protected void doClientStop()
+    {
+        ShutdownThread.deregister(this);
+    }
 }

jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java

@@ -296,4 +296,16 @@ protected void doStart() throws Exception
             deferredEndpointConfigs.clear();
         }
     }
+
+    @Override
+    protected void doClientStart()
+    {
+        // Do nothing to avoid registration with the ShutdownThread.
+    }
+
+    @Override
+    protected void doClientStop()
+    {
+        // Do nothing to avoid de-registration with the ShutdownThread.
+    }
 }

jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/ClientInWebappTest.java

@@ -0,0 +1,108 @@
+//
+// ========================================================================
+// 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 v. 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+// ========================================================================
+//
+
+package org.eclipse.jetty.websocket.javax.tests;
+
+import java.io.IOException;
+import java.net.URI;
+import java.util.Collection;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.websocket.ContainerProvider;
+import javax.websocket.WebSocketContainer;
+
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.eclipse.jetty.websocket.javax.client.internal.JavaxWebSocketClientContainer;
+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.empty;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+public class ClientInWebappTest
+{
+    private Server server;
+    private ServletContextHandler contextHandler;
+    private URI serverUri;
+    private HttpClient httpClient;
+    private volatile WebSocketContainer container;
+
+    public class WebSocketClientInServlet extends HttpServlet
+    {
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+        {
+            container = ContainerProvider.getWebSocketContainer();
+        }
+    }
+
+    @BeforeEach
+    public void before() throws Exception
+    {
+        server = new Server();
+        ServerConnector connector = new ServerConnector(server);
+        connector.setPort(8080);
+        server.addConnector(connector);
+
+        contextHandler = new ServletContextHandler();
+        contextHandler.setContextPath("/");
+        contextHandler.addServlet(new ServletHolder(new WebSocketClientInServlet()), "/");
+        server.setHandler(contextHandler);
+        server.start();
+        serverUri = WSURI.toWebsocket(server.getURI());
+
+        httpClient = new HttpClient();
+        httpClient.start();
+    }
+
+    @AfterEach
+    public void after() throws Exception
+    {
+        httpClient.stop();
+        server.stop();
+    }
+
+    @Test
+    public void testWebSocketClientContainerInWebapp() throws Exception
+    {
+        ContentResponse response = httpClient.GET(serverUri);
+        assertThat(response.getStatus(), is(HttpStatus.OK_200));
+
+        assertNotNull(container);
+        assertThat(container, instanceOf(JavaxWebSocketClientContainer.class));
+        JavaxWebSocketClientContainer clientContainer = (JavaxWebSocketClientContainer)container;
+        assertThat(clientContainer.isRunning(), is(true));
+
+        // The container should be a bean on the ContextHandler.
+        Collection<WebSocketContainer> containedBeans = contextHandler.getBeans(WebSocketContainer.class);
+        assertThat(containedBeans.size(), is(1));
+        assertThat(containedBeans.toArray()[0], is(container));
+
+        // The client should be attached to the servers LifeCycle and should stop with it.
+        server.stop();
+        assertThat(clientContainer.isRunning(), is(false));
+        assertThat(server.getContainedBeans(WebSocketContainer.class), empty());
+    }
+}