From 51c5a4c833f1ada8629de3daa0a393bd80c13829 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 5 May 2014 14:53:53 +0200 Subject: [PATCH] 425421 ContainerLifeCycle does not start added beans in started state --- .../org/eclipse/jetty/client/HttpClient.java | 2 +- .../server/handler/HandlerCollection.java | 4 +- .../jetty/server/handler/HandlerWrapper.java | 3 +- .../eclipse/jetty/servlet/ServletHandler.java | 1 - .../util/component/ContainerLifeCycle.java | 52 ++++++++++++++++--- .../jetty/util/thread/QueuedThreadPool.java | 2 + .../component/ContainerLifeCycleTest.java | 30 +++++++---- .../jsr356/server/BasicEndpointTest.java | 1 - .../server/ExtensionStackProcessingTest.java | 4 +- .../websocket/jsr356/server/WSServer.java | 2 + ...asicEchoEndpointConfigContextListener.java | 3 ++ .../websocket/client/WebSocketClient.java | 20 ++----- .../client/io/UpgradeConnection.java | 2 +- 13 files changed, 84 insertions(+), 42 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 5649b2992ee..8e323bc734a 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -201,8 +201,8 @@ public class HttpClient extends ContainerLifeCycle scheduler = new ScheduledExecutorScheduler(name + "-scheduler", false); addBean(scheduler); - addBean(transport); transport.setHttpClient(this); + addBean(transport); resolver = new SocketAddressResolver(executor, scheduler, getAddressResolutionTimeout()); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerCollection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerCollection.java index aacfaf39f2e..c118c7ec1da 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerCollection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerCollection.java @@ -85,9 +85,9 @@ public class HandlerCollection extends AbstractHandlerContainer for (Handler handler:handlers) if (handler.getServer()!=getServer()) handler.setServer(getServer()); - - updateBeans(_handlers, handlers); + Handler[] old=_handlers;; _handlers = handlers; + updateBeans(old, handlers); } /* ------------------------------------------------------------ */ diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerWrapper.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerWrapper.java index e91c824421a..dcbf6ff5d54 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerWrapper.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/HandlerWrapper.java @@ -84,8 +84,9 @@ public class HandlerWrapper extends AbstractHandlerContainer if (handler!=null) handler.setServer(getServer()); - updateBean(_handler,handler); + Handler old=_handler; _handler=handler; + updateBean(old,_handler); } /* ------------------------------------------------------------ */ diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index 6f0e5b170fb..f19bcd9eb0b 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -814,7 +814,6 @@ public class ServletHandler extends ScopedHandler /* ------------------------------------------------------------ */ /** Initialize filters and load-on-startup servlets. - * Called automatically from start if autoInitializeServlet is true. */ public void initialize() throws Exception diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java index 3fe7c448929..464c0f7e4a8 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/component/ContainerLifeCycle.java @@ -38,8 +38,11 @@ import org.eclipse.jetty.util.log.Logger; *

* When a {@link LifeCycle} bean is added without a managed state being specified the state is determined heuristically: *

* When the container is started, then all contained managed beans will also be started. Any contained Auto beans * will be check for their status and if already started will be switched unmanaged beans, else they will be @@ -64,13 +67,17 @@ import org.eclipse.jetty.util.log.Logger; * +? referenced AUTO object that could become MANAGED or UNMANAGED. * */ + +/* ------------------------------------------------------------ */ +/** + */ @ManagedObject("Implementation of Container and LifeCycle") public class ContainerLifeCycle extends AbstractLifeCycle implements Container, Destroyable, Dumpable { private static final Logger LOG = Log.getLogger(ContainerLifeCycle.class); private final List _beans = new CopyOnWriteArrayList<>(); private final List _listeners = new CopyOnWriteArrayList<>(); - private boolean _started = false; + private boolean _doStarted = false; public ContainerLifeCycle() @@ -84,7 +91,7 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, protected void doStart() throws Exception { // indicate that we are started, so that addBean will start other beans added. - _started = true; + _doStarted = true; // start our managed and auto beans for (Bean b : _beans) @@ -142,7 +149,7 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, @Override protected void doStop() throws Exception { - _started = false; + _doStarted = false; super.doStop(); List reverse = new ArrayList<>(_beans); Collections.reverse(reverse); @@ -267,7 +274,7 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, case MANAGED: manage(new_bean); - if (_started) + if (isStarting() && _doStarted) { LifeCycle l = (LifeCycle)o; if (!l.isRunning()) @@ -279,16 +286,20 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, if (o instanceof LifeCycle) { LifeCycle l = (LifeCycle)o; - if (_started) + if (isStarting()) { if (l.isRunning()) unmanage(new_bean); - else + else if (_doStarted) { manage(new_bean); start(l); } + else + new_bean._managed=Managed.AUTO; } + else if (isStarted()) + unmanage(new_bean); else new_bean._managed=Managed.AUTO; } @@ -315,6 +326,31 @@ public class ContainerLifeCycle extends AbstractLifeCycle implements Container, } + /* ------------------------------------------------------------ */ + /** Add a managed lifecycle. + *

This is a conveniance method that uses addBean(lifecycle,true) + * and then ensures that the added bean is started iff this container + * is running. Exception from nested calls to start are caught and + * wrapped as RuntimeExceptions + * @param lifecycle + */ + public void addManaged(LifeCycle lifecycle) + { + addBean(lifecycle,true); + try + { + if (isRunning() && !lifecycle.isRunning()) + start(lifecycle); + } + catch (RuntimeException | Error e) + { + throw e; + } + catch (Exception e) + { + throw new RuntimeException(e); + } + } @Override public void addEventListener(Container.Listener listener) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java index 1918ad15c73..5cc75121c0b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java @@ -661,4 +661,6 @@ public class QueuedThreadPool extends AbstractLifeCycle implements SizedThreadPo } return null; } + + } diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java index 0eb37a4fd13..ddc152be381 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/component/ContainerLifeCycleTest.java @@ -108,7 +108,16 @@ public class ContainerLifeCycleTest Assert.assertEquals(1,destroyed.get()); a0.addBean(a1); + Assert.assertEquals(2,started.get()); + Assert.assertEquals(2,stopped.get()); + Assert.assertEquals(1,destroyed.get()); + Assert.assertFalse(a0.isManaged(a1)); a0.start(); + Assert.assertEquals(2,started.get()); + Assert.assertEquals(2,stopped.get()); + Assert.assertEquals(1,destroyed.get()); + a1.start(); + a0.manage(a1); Assert.assertEquals(3,started.get()); Assert.assertEquals(2,stopped.get()); Assert.assertEquals(1,destroyed.get()); @@ -285,16 +294,16 @@ public class ContainerLifeCycleTest dump=trim(a0.dump()); dump=check(dump,"org.eclipse.jetty.util.component.ContainerLifeCycl"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); - dump=check(dump," | += org.eclipse.jetty.util.component.Container"); + dump=check(dump," | +~ org.eclipse.jetty.util.component.Container"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); dump=check(dump,""); ContainerLifeCycle aa10 = new ContainerLifeCycle(); - aa1.addBean(aa10); + aa1.addBean(aa10,true); dump=trim(a0.dump()); dump=check(dump,"org.eclipse.jetty.util.component.ContainerLifeCycl"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); - dump=check(dump," | += org.eclipse.jetty.util.component.Container"); + dump=check(dump," | +~ org.eclipse.jetty.util.component.Container"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); dump=check(dump," += org.eclipse.jetty.util.component.Container"); dump=check(dump,""); @@ -314,11 +323,11 @@ public class ContainerLifeCycleTest dump(out,indent,TypeUtil.asList(new Object[]{a1,a2}),TypeUtil.asList(new Object[]{a3,a4})); } }; - a0.addBean(aa); + a0.addBean(aa,true); dump=trim(a0.dump()); dump=check(dump,"org.eclipse.jetty.util.component.ContainerLifeCycl"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); - dump=check(dump," | += org.eclipse.jetty.util.component.Container"); + dump=check(dump," | +~ org.eclipse.jetty.util.component.Container"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); dump=check(dump," | += org.eclipse.jetty.util.component.Container"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); @@ -332,14 +341,14 @@ public class ContainerLifeCycleTest dump=trim(a0.dump()); dump=check(dump,"org.eclipse.jetty.util.component.ContainerLifeCycl"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); - dump=check(dump," | += org.eclipse.jetty.util.component.Container"); + dump=check(dump," | +~ org.eclipse.jetty.util.component.Container"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); dump=check(dump," | += org.eclipse.jetty.util.component.Container"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); dump=check(dump," +- org.eclipse.jetty.util.component.Container"); dump=check(dump," +- org.eclipse.jetty.util.component.Container"); dump=check(dump," | += org.eclipse.jetty.util.component.Conta"); - dump=check(dump," | += org.eclipse.jetty.util.component.C"); + dump=check(dump," | +~ org.eclipse.jetty.util.component.C"); dump=check(dump," +- org.eclipse.jetty.util.component.Container"); dump=check(dump," +- org.eclipse.jetty.util.component.Container"); dump=check(dump,""); @@ -348,7 +357,7 @@ public class ContainerLifeCycleTest dump=trim(a0.dump()); dump=check(dump,"org.eclipse.jetty.util.component.ContainerLifeCycl"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); - dump=check(dump," | += org.eclipse.jetty.util.component.Container"); + dump=check(dump," | +~ org.eclipse.jetty.util.component.Container"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); dump=check(dump," | += org.eclipse.jetty.util.component.Container"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); @@ -363,7 +372,7 @@ public class ContainerLifeCycleTest dump=trim(a0.dump()); dump=check(dump,"org.eclipse.jetty.util.component.ContainerLifeCycl"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); - dump=check(dump," | += org.eclipse.jetty.util.component.Container"); + dump=check(dump," | +~ org.eclipse.jetty.util.component.Container"); dump=check(dump," += org.eclipse.jetty.util.component.ContainerLife"); dump=check(dump," | += org.eclipse.jetty.util.component.Container"); dump=check(dump," +~ org.eclipse.jetty.util.component.ContainerLife"); @@ -523,7 +532,10 @@ public class ContainerLifeCycleTest c0.addBean(c00); c0.start(); c0.addBean(inherited); + c0.manage(inherited); c0.addBean(c01); + c01.start(); + c0.manage(c01); Assert.assertTrue(c0.isManaged(inherited)); Assert.assertFalse(c00.isManaged(inherited)); diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/BasicEndpointTest.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/BasicEndpointTest.java index 03359878e97..10ac6e56abd 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/BasicEndpointTest.java +++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/BasicEndpointTest.java @@ -64,7 +64,6 @@ public class BasicEndpointTest WebAppContext webapp = wsb.createWebAppContext(); wsb.deployWebapp(webapp); - // wsb.dump(); WebSocketClient client = new WebSocketClient(bufferPool); try diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/ExtensionStackProcessingTest.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/ExtensionStackProcessingTest.java index d2fbcd7c853..82f094e74af 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/ExtensionStackProcessingTest.java +++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/ExtensionStackProcessingTest.java @@ -68,10 +68,10 @@ public class ExtensionStackProcessingTest ServerEndpointConfig config = ServerEndpointConfig.Builder.create(BasicEchoEndpoint.class, "/").build(); container.addEndpoint(config); - server.start(); - client = ContainerProvider.getWebSocketContainer(); server.addBean(client, true); + + server.start(); } @After diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/WSServer.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/WSServer.java index 00813197b58..9297c7f3d2f 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/WSServer.java +++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/WSServer.java @@ -110,6 +110,7 @@ public class WSServer WebAppContext context = new WebAppContext(); context.setContextPath(this.contextPath); context.setBaseResource(Resource.newResource(this.contextDir)); + context.setAttribute("org.eclipse.jetty.websocket.jsr356",Boolean.TRUE); // @formatter:off context.setConfigurations(new Configuration[] { @@ -133,6 +134,7 @@ public class WSServer public void deployWebapp(WebAppContext webapp) throws Exception { contexts.addHandler(webapp); + contexts.manage(webapp); webapp.start(); if (LOG.isDebugEnabled()) { diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/samples/echo/BasicEchoEndpointConfigContextListener.java b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/samples/echo/BasicEchoEndpointConfigContextListener.java index 05b239f4b88..e224de8ab1f 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/samples/echo/BasicEchoEndpointConfigContextListener.java +++ b/jetty-websocket/javax-websocket-server-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/samples/echo/BasicEchoEndpointConfigContextListener.java @@ -39,6 +39,9 @@ public class BasicEchoEndpointConfigContextListener implements ServletContextLis public void contextInitialized(ServletContextEvent sce) { ServerContainer container = (ServerContainer)sce.getServletContext().getAttribute(ServerContainer.class.getName()); + if (container==null) + throw new IllegalStateException("No Websocket ServerContainer in "+sce.getServletContext()); + // Build up a configuration with a specific path String path = "/echo"; ServerEndpointConfig.Builder builder = ServerEndpointConfig.Builder.create(BasicEchoEndpoint.class,path); diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index 387874a52ec..8ab24ec9063 100644 --- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java @@ -36,6 +36,7 @@ import org.eclipse.jetty.io.SelectorManager; import org.eclipse.jetty.util.HttpCookieStore; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.ssl.SslContextFactory; @@ -430,30 +431,17 @@ public class WebSocketClient extends ContainerLifeCycle implements SessionListen threadPool.setName(name); threadPool.setDaemon(daemon); executor = threadPool; - addBean(executor,true); + addManaged(threadPool); } else { addBean(executor,false); } - if (connectionManager != null) - { - return; - } - try + if (connectionManager == null) { connectionManager = newConnectionManager(); - addBean(connectionManager); - connectionManager.start(); - } - catch (IOException e) - { - throw e; - } - catch (Exception e) - { - throw new IOException(e); + addManaged(connectionManager); } } diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/UpgradeConnection.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/UpgradeConnection.java index 9bc359b1daf..fe385d3756e 100644 --- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/UpgradeConnection.java +++ b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/UpgradeConnection.java @@ -260,7 +260,7 @@ public class UpgradeConnection extends AbstractConnection extensionStack.setNextOutgoing(connection); session.addBean(extensionStack); - connectPromise.getClient().addBean(session); + connectPromise.getClient().addManaged(session); // Now swap out the connection endp.setConnection(connection);