425421 ContainerLifeCycle does not start added beans in started state

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 5649b2992e..8e323bc734 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 aacfaf39f2..c118c7ec1d 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 e91c824421..dcbf6ff5d5 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 6f0e5b170f..f19bcd9eb0 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 3fe7c44892..464c0f7e4a 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;
  * <p>
  * When a {@link LifeCycle} bean is added without a managed state being specified the state is determined heuristically:
  * <ul>
- *   <li>If when added, a bean is already started, then it is assumed to be an unmanaged bean.
- *   <li>If when added, a bean is not started, then it is added in Auto state.
+ *   <li>If the added bean is running, it will be added as an unmanaged bean.
+ *   <li>If the added bean is !running and the container is !running, it will be added as an AUTO bean (see below).
+ *   <li>If the added bean is !running and the container is starting, it will be added as an managed bean and will be started (this handles the frequent case of 
+ *   new beans added during calls to doStart).
+ *   <li>If the added bean is !running and the container is started, it will be added as an unmanaged bean.
  * </ul>
  * 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.
  * </pre>
  */
+
+/* ------------------------------------------------------------ */
+/**
+ */
 @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<Bean> _beans = new CopyOnWriteArrayList<>();
     private final List<Container.Listener> _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<Bean> 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.
+     * <p>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 1918ad15c7..5cc75121c0 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 0eb37a4fd1..ddc152be38 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 03359878e9..10ac6e56ab 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 d2fbcd7c85..82f094e74a 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 00813197b5..9297c7f3d2 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 05b239f4b8..e224de8ab1 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 387874a52e..8ab24ec906 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 9bc359b1da..fe385d3756 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);