Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
97bbec7
Issue #5832 - deregister ShutdownThread for WebSocketClientContainer
lachlan-roberts Dec 22, 2020
374e02c
register ShutdownThread in doStart
lachlan-roberts Dec 22, 2020
25f8c65
Issue #5832 - shutdown WSClientContainer with ContextHandler if possible
lachlan-roberts Dec 23, 2020
dd1d4bc
Issue #5832 - use lifeCycleStopping as lifeCycleStopped is never called
lachlan-roberts Dec 23, 2020
3286f9a
Issue #5832 - changes from review
lachlan-roberts Dec 24, 2020
774dac6
Allow shutdown of Javax WS Client Container though a SCI
lachlan-roberts Jan 8, 2021
9e19e87
Issue #5832 - deprecate and remove usage of the ContainerInitializer …
lachlan-roberts Jan 12, 2021
35051df
Issue #5832 - changes from review
lachlan-roberts Jan 13, 2021
2f61766
improve testing for JavaxWebSocketClientContainer shutdown
lachlan-roberts Jan 14, 2021
29f1859
Merge remote-tracking branch 'origin/jetty-10.0.x' into jetty-10.0.x-…
lachlan-roberts Jan 18, 2021
a60ecfa
Issue #5832 - fix bugs when stopping the JavaxWebSocketClientContainer
lachlan-roberts Jan 18, 2021
99aef1a
Issue #5832 - fix bugs when stopping the JavaxWebSocketClientContainer
lachlan-roberts Jan 18, 2021
c6c1ccf
Rename JavaxWebSocketClientContainer.initialize to setShutdownContainer.
lachlan-roberts Jan 20, 2021
ff4f2ef
only allow adding ServletContainerInitializers before starting
lachlan-roberts Jan 21, 2021
7760d04
Merge remote-tracking branch 'origin/jetty-10.0.x' into jetty-10.0.x-…
lachlan-roberts Feb 21, 2021
ed86361
Revert changes to ServletContainerInitializer
lachlan-roberts Feb 21, 2021
47f24db
Issue #5832 - Improve testing for WebSocket client shutdown.
lachlan-roberts Feb 22, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -696,6 +696,27 @@ else if (handler instanceof ServletHandler)
relinkHandlers();
}

/**
* Utility Method to allow for manual execution of {@link javax.servlet.ServletContainerInitializer} when using Embedded Jetty.
* @param containerInitializer the ServletContainerInitializer to register.
* @see Initializer
*/
public void addServletContainerInitializer(ServletContainerInitializer containerInitializer)
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
{
addManaged(new Initializer(this, containerInitializer));
}

/**
* Utility Method to allow for manual execution of {@link javax.servlet.ServletContainerInitializer} when using Embedded Jetty.
* @param containerInitializer the ServletContainerInitializer to register.
* @param classes the Set of application classes.
* @see Initializer
*/
public void addServletContainerInitializer(ServletContainerInitializer containerInitializer, Set<Class<?>> classes)
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
{
addManaged(new Initializer(this, containerInitializer, classes));
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* The DecoratedObjectFactory for use by IoC containers (weld / spring / etc)
*
Expand Down
Expand Up @@ -25,7 +25,9 @@
/**
* Utility Methods for manual execution of {@link javax.servlet.ServletContainerInitializer} when
* using Embedded Jetty.
* @deprecated use {@link org.eclipse.jetty.servlet.ServletContextHandler#addServletContainerInitializer(ServletContainerInitializer)}.
*/
@Deprecated
public final class ContainerInitializer
{
/**
Expand Down
5 changes: 5 additions & 0 deletions jetty-websocket/websocket-javax-client/pom.xml
Expand Up @@ -34,6 +34,11 @@
<artifactId>jetty-client</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.toolchain</groupId>
<artifactId>jetty-servlet-api</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-xml</artifactId>
Expand Down
Expand Up @@ -20,6 +20,8 @@
exports org.eclipse.jetty.websocket.javax.client;
exports org.eclipse.jetty.websocket.javax.client.internal to org.eclipse.jetty.websocket.javax.server;

requires static jetty.servlet.api;
requires org.slf4j;
requires org.eclipse.jetty.client;
requires org.eclipse.jetty.websocket.core.client;
requires org.eclipse.jetty.websocket.javax.common;
Expand Down
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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;
}
}
@@ -0,0 +1,48 @@
//
// ========================================================================
// 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.client;

import java.util.Set;
import javax.servlet.ServletContainerInitializer;
import javax.servlet.ServletContext;
import javax.servlet.ServletContextEvent;
import javax.servlet.ServletContextListener;
import javax.servlet.ServletException;

import org.eclipse.jetty.util.component.ContainerLifeCycle;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.websocket.javax.client.internal.JavaxWebSocketClientContainer;

public class JavaxWebSocketClientShutdown extends ContainerLifeCycle implements ServletContainerInitializer, ServletContextListener
{
@Override
public void onStartup(Set<Class<?>> c, ServletContext ctx) throws ServletException
{
JavaxWebSocketClientContainer.SHUTDOWN_CONTAINER.compareAndSet(null, this);
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
ctx.addListener(this);
}

@Override
public void contextInitialized(ServletContextEvent sce)
{
LifeCycle.start(this);
}

@Override
public void contextDestroyed(ServletContextEvent sce)
{
LifeCycle.stop(this);
removeBeans();
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
}
}
Expand Up @@ -15,13 +15,15 @@

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;
import java.util.concurrent.Executor;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import javax.websocket.ClientEndpoint;
import javax.websocket.ClientEndpointConfig;
Expand All @@ -33,6 +35,9 @@

import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.util.annotation.ManagedObject;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
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;
Expand All @@ -43,6 +48,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.
Expand All @@ -52,6 +59,9 @@
@ManagedObject("JSR356 Client Container")
public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer implements javax.websocket.WebSocketContainer
{
private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketClientContainer.class);
public static final AtomicReference<ContainerLifeCycle> SHUTDOWN_CONTAINER = new AtomicReference<>();
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved

protected WebSocketCoreClient coreClient;
protected Function<WebSocketComponents, WebSocketCoreClient> coreClientFactory;
private final JavaxWebSocketClientFrameHandlerFactory frameHandlerFactory;
Expand Down Expand Up @@ -271,4 +281,75 @@ private ClientEndpointConfig getAnnotatedConfig(Object endpoint) throws Deployme

return new AnnotatedClientEndpointConfig(anno);
}

@Override
protected void doStart() throws Exception
{
doClientStart();
super.doStart();
}

protected void doClientStart()
{
// If we are running in Jetty register shutdown with the ContextHandler.
// TODO: add test mode to disable this.
if (shutdownWithContextHandler(this))
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
return;

// If we are running inside a different ServletContainer we can register with the SHUTDOWN_CONTAINER static.
ContainerLifeCycle shutdownContainer = SHUTDOWN_CONTAINER.get();
if (shutdownContainer != null)
{
shutdownContainer.addManaged(this);
return;
}

ShutdownThread.register(this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment here

}

private boolean shutdownWithContextHandler(LifeCycle lifeCycle)
{
try
{
Object context = getClass().getClassLoader()
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
.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)
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
.invoke(contextHandler, lifeCycle);

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);
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
return true;
}
catch (Throwable throwable)
{
return false;
}
}
}
@@ -0,0 +1 @@
org.eclipse.jetty.websocket.javax.client.JavaxWebSocketClientShutdown
Expand Up @@ -15,6 +15,7 @@

import java.util.HashSet;
import java.util.Set;
import java.util.function.Consumer;
import javax.servlet.ServletContainerInitializer;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
Expand All @@ -28,7 +29,6 @@

import org.eclipse.jetty.servlet.FilterHolder;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.listener.ContainerInitializer;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.thread.ThreadClassLoaderScope;
import org.eclipse.jetty.websocket.core.WebSocketComponents;
Expand All @@ -52,6 +52,8 @@ public class JavaxWebSocketServletContainerInitializer implements ServletContain
public static final String HTTPCLIENT_ATTRIBUTE = "org.eclipse.jetty.websocket.javax.HttpClient";
private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketServletContainerInitializer.class);

private Consumer<ServletContext> afterStartupConsumer;

/**
* Test a ServletContext for {@code init-param} or {@code attribute} at {@code keyName} for
* true or false setting that determines if the specified feature is enabled (or not).
Expand Down Expand Up @@ -102,7 +104,7 @@ public static void configure(ServletContextHandler context, Configurator configu
// the initialization phase is over. (important for this SCI to function)
context.getServletContext().setExtendedListenerTypes(true);

context.addEventListener(ContainerInitializer.asContextListener(new JavaxWebSocketServletContainerInitializer())
context.addServletContainerInitializer(new JavaxWebSocketServletContainerInitializer()
.afterStartup((servletContext) ->
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
{
JavaxWebSocketServerContainer serverContainer = JavaxWebSocketServerContainer.getContainer(servletContext);
Expand Down Expand Up @@ -270,6 +272,9 @@ public void onStartup(Set<Class<?>> c, ServletContext context) throws ServletExc
}
}
}

if (afterStartupConsumer != null)
afterStartupConsumer.accept(context);
}

@SuppressWarnings("unchecked")
Expand All @@ -296,4 +301,17 @@ private void filterClasses(Set<Class<?>> c, Set<Class<? extends Endpoint>> disco
}
}
}

/**
* Add a optional consumer to execute once the {@link ServletContainerInitializer#onStartup(Set, ServletContext)} has
* been called successfully.
*
* @param consumer the consumer to execute after the SCI has executed
* @return this configured {@link JavaxWebSocketServletContainerInitializer} instance.
*/
public JavaxWebSocketServletContainerInitializer afterStartup(Consumer<ServletContext> consumer)
{
this.afterStartupConsumer = consumer;
return this;
}
}
Expand Up @@ -296,4 +296,10 @@ protected void doStart() throws Exception
deferredEndpointConfigs.clear();
}
}

@Override
protected void doClientStart()
{
// Do nothing to avoid registration with the ShutdownThread.
}
}