Open Network Connectors before starting contexts.

jetty-server/src/main/config/etc/jetty.xml

@@ -126,5 +126,6 @@
     <Set name="stopTimeout"><Property name="jetty.server.stopTimeout" default="5000"/></Set>
     <Set name="dumpAfterStart"><Property name="jetty.server.dumpAfterStart" deprecated="jetty.dump.start" default="false"/></Set>
     <Set name="dumpBeforeStop"><Property name="jetty.server.dumpBeforeStop" deprecated="jetty.dump.stop" default="false"/></Set>
+    <Set name="verifiedStartSequence"><Property name="jetty.server.verifiedStartSequence" default="false"/></Set>
 
 </Configure>

jetty-server/src/main/config/modules/server.mod

@@ -82,3 +82,6 @@ etc/jetty.xml
 
 ## Dump the state of the Jetty server, components, and webapps before shutdown
 # jetty.server.dumpBeforeStop=false
+
+## Verified Start Sequence by checking ports and contexts are available before starting connectors
+# jetty.server.verifiedStartSequence=false

jetty-server/src/main/java/org/eclipse/jetty/server/Server.java

@@ -85,6 +85,7 @@ public class Server extends HandlerWrapper implements Attributes
     private boolean _stopAtShutdown;
     private boolean _dumpAfterStart=false;
     private boolean _dumpBeforeStop=false;
+    private boolean _verifiedStartSequence=false;
     private ErrorHandler _errorHandler;
     private RequestLog _requestLog;
 
@@ -136,6 +137,33 @@ public Server(@Name("threadpool") ThreadPool pool)
         setServer(this);
     }
 
+    /**
+     * @return True if the start sequence is verified by checking port availability prior
+     * to starting contexts and checking contexts prior to starting connectors.
+     */
+    public boolean isVerifiedStartSequence()
+    {
+        return _verifiedStartSequence;
+    }
+
+    /**
+     * Set if the start sequence is to be verified by in the following stages:
+     * <ul>
+     * <li>{@link NetworkConnector}s are opened with {@link NetworkConnector#open()} to check ports are
+     * available and to reserve them.</li>
+     * <li>Start the contained {@link Handler}s and beans.</li>
+     * <li>Start the {@link Connector}s
+     * </ul>
+     * If any failures are encountered at any of the stages, the start sequence is aborted without 
+     * running the subsequent stage and reversing closing any open connectors.
+     * @param verified If true the start sequence is verified by checking port availability prior
+     * to starting contexts and checking contexts prior to starting connectors.
+     */
+    public void setVerifiedStartSequence(boolean verified)
+    {
+        _verifiedStartSequence = verified;
+    }
+
     /* ------------------------------------------------------------ */
     public RequestLog getRequestLog()
     {
@@ -346,68 +374,120 @@ public HttpField getDateField()
     @Override
     protected void doStart() throws Exception
     {
-        // Create an error handler if there is none
-        if (_errorHandler==null)
-            _errorHandler=getBean(ErrorHandler.class);
-        if (_errorHandler==null)
-            setErrorHandler(new ErrorHandler());
-        if (_errorHandler instanceof ErrorHandler.ErrorPageMapper)
-            LOG.warn("ErrorPageMapper not supported for Server level Error Handling");
-        _errorHandler.setServer(this);
-
-        //If the Server should be stopped when the jvm exits, register
-        //with the shutdown handler thread.
-        if (getStopAtShutdown())
-            ShutdownThread.register(this);
-
-        //Register the Server with the handler thread for receiving
-        //remote stop commands
-        ShutdownMonitor.register(this);
-
-        //Start a thread waiting to receive "stop" commands.
-        ShutdownMonitor.getInstance().start(); // initialize
+        try
+        {
+            // Create an error handler if there is none
+            if (_errorHandler==null)
+                _errorHandler=getBean(ErrorHandler.class);
+            if (_errorHandler==null)
+                setErrorHandler(new ErrorHandler());
+            if (_errorHandler instanceof ErrorHandler.ErrorPageMapper)
+                LOG.warn("ErrorPageMapper not supported for Server level Error Handling");
+            _errorHandler.setServer(this);
+
+            //If the Server should be stopped when the jvm exits, register
+            //with the shutdown handler thread.
+            if (getStopAtShutdown())
+                ShutdownThread.register(this);
+
+            //Register the Server with the handler thread for receiving
+            //remote stop commands
+            ShutdownMonitor.register(this);
+
+            //Start a thread waiting to receive "stop" commands.
+            ShutdownMonitor.getInstance().start(); // initialize
+
+            String gitHash = Jetty.GIT_HASH;
+            String timestamp = Jetty.BUILD_TIMESTAMP;
+
+            LOG.info("jetty-{}, build timestamp: {}, git hash: {}", getVersion(), timestamp, gitHash);
+            if (!Jetty.STABLE)
+            {
+                LOG.warn("THIS IS NOT A STABLE RELEASE! DO NOT USE IN PRODUCTION!");
+                LOG.warn("Download a stable release from http://download.eclipse.org/jetty/");
+            }
 
-        String gitHash = Jetty.GIT_HASH;
-        String timestamp = Jetty.BUILD_TIMESTAMP;
+            HttpGenerator.setJettyVersion(HttpConfiguration.SERVER_VERSION);
 
-        LOG.info("jetty-{}, build timestamp: {}, git hash: {}", getVersion(), timestamp, gitHash);
-        if (!Jetty.STABLE)
-        {
-            LOG.warn("THIS IS NOT A STABLE RELEASE! DO NOT USE IN PRODUCTION!");
-            LOG.warn("Download a stable release from http://download.eclipse.org/jetty/");
-        }
-
-        HttpGenerator.setJettyVersion(HttpConfiguration.SERVER_VERSION);
+            MultiException mex=new MultiException();
 
-        MultiException mex=new MultiException();
-        try
-        {
-            super.doStart();
-        }
-        catch(Throwable e)
-        {
-            mex.add(e);
-        }
+            // Open network connectors first
+            if (_verifiedStartSequence)
+            {
+                _connectors.stream().filter(NetworkConnector.class::isInstance).map(NetworkConnector.class::cast).forEach(connector->
+                {
+                    try
+                    {
+                        connector.open();
+                    }
+                    catch(Throwable e)
+                    {
+                        mex.add(e);
+                    }
+                });
+
+                // Throw now if verified start sequence and there was an open exception
+                mex.ifExceptionThrow();
+            }
 
-        // start connectors last
-        for (Connector connector : _connectors)
-        {
             try
             {
-                connector.start();
+                // Start the server and components, but #start(LifeCycle) is overridden
+                // so that connectors are not started until after all other components are 
+                // started.
+                super.doStart();
             }
             catch(Throwable e)
             {
                 mex.add(e);
+
+                // Throw now if verified start sequence and there was a doStart exception
+                if (_verifiedStartSequence)
+                    mex.ifExceptionThrow();
+            }
+
+            // start connectors last
+            for (Connector connector : _connectors)
+            {
+                try
+                {
+                    connector.start();
+                }
+                catch(Throwable e)
+                {
+                    mex.add(e);
+                }
             }
+
+            mex.ifExceptionThrow();
+            LOG.info(String.format("Started @%dms",Uptime.getUptime()));
         }
+        catch(Throwable x)
+        { 
+            if (_verifiedStartSequence)
+            {
+                // stop any connectors that were opened
+                _connectors.stream().filter(NetworkConnector.class::isInstance).map(NetworkConnector.class::cast).forEach(NetworkConnector::close);
 
-        if (isDumpAfterStart())
-            dumpStdErr();
+                // Stop our components (including connectors
+                try
+                {
+                    super.doStop();
+                }
+                catch(Throwable e)
+                {
+                    x.addSuppressed(e);
+                }
+            }
 
-        mex.ifExceptionThrow();
+            throw x;
+        }
+        finally
+        {
+            if (isDumpAfterStart())
+                dumpStdErr();
+        }
 
-        LOG.info(String.format("Started @%dms",Uptime.getUptime()));
     }
 
     @Override

jetty-server/src/test/java/org/eclipse/jetty/server/NotAcceptingTest.java

@@ -18,14 +18,21 @@
 
 package org.eclipse.jetty.server;
 
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 
 import java.io.IOException;
+import java.net.BindException;
+import java.net.ServerSocket;
 import java.net.Socket;
 import java.util.concurrent.Exchanger;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -36,6 +43,7 @@
 import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.eclipse.jetty.toolchain.test.AdvancedRunner;
 import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.thread.QueuedThreadPool;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -376,6 +384,136 @@ public void testConnectionLimit() throws Exception
         assertThat(asyncConnector.isAccepting(),is(true));
 
     }
+
+    @Test
+    public void testVerifiedStartSequenceAlreadyOpened() throws Exception
+    {
+        server.setVerifiedStartSequence(true);
+
+        try(ServerSocket alreadyOpened = new ServerSocket(0))
+        {
+            // Add a connector that can be opened
+            AtomicInteger opened0 = new AtomicInteger(0);
+            ServerConnector connector0 = new ServerConnector(server)
+            {
+                @Override
+                public void open() throws IOException
+                {
+                    super.open();
+                    opened0.set(getLocalPort());
+                }
+
+            };
+            server.addConnector(connector0);
+
+            // Add a connector that will fail to open with port in use
+            ServerConnector connector1 = new ServerConnector(server);
+            connector1.setPort(alreadyOpened.getLocalPort());
+            server.addConnector(connector1);
+
+            // Add a handler to detect if handlers are started
+            AtomicBoolean handlerStarted = new AtomicBoolean(false);
+            server.setHandler(new AbstractHandler()
+            {
+                @Override
+                public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response)
+                        throws IOException, ServletException
+                {                    
+                }
+
+                @Override
+                public void doStart()
+                {
+                    handlerStarted.set(true);
+                }
+            });
+
+
+            // try to start the server
+            try
+            {
+                server.start();
+                Assert.fail();
+            }
+            catch (BindException e)
+            {
+                // expected
+                assertThat(e.getMessage(),containsString("Address already in use"));
+            }
+
+            // Check that connector0 was opened OK
+            assertThat(opened0.get(),greaterThan(0));
+            // and it is now closed
+            assertFalse(connector0.isOpen());
+            assertFalse(connector0.isRunning());
+
+            // Check that handlers were never started
+            assertFalse(handlerStarted.get());
+
+            // Check that server components were stopped
+            assertFalse(server.getBean(QueuedThreadPool.class).isStarted());
+        }
+    }
+
+    @Test
+    public void testVerifiedStartSequenceBadHandler() throws Exception
+    {
+        server.setVerifiedStartSequence(true);
+
+        // Add a connector that can be opened
+        AtomicInteger opened = new AtomicInteger(0);
+        ServerConnector connector = new ServerConnector(server)
+        {
+            @Override
+            public void open() throws IOException
+            {
+                super.open();
+                opened.set(getLocalPort());
+            }
+
+        };
+        server.addConnector(connector);
+
+
+        // Add a handler that will fail to start
+        server.setHandler(new AbstractHandler()
+        {
+            @Override
+            public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response)
+                    throws IOException, ServletException
+            {                    
+            }
+
+            @Override
+            public void doStart() throws Exception
+            {
+                throw new Exception("Expected failure during start");
+            }
+        });
+
+
+        // try to start the server
+        try
+        {
+            server.start();
+            Assert.fail();
+        }
+        catch (Exception e)
+        {
+            // expected
+            assertThat(e.getMessage(),containsString("Expected failure during start"));
+        }
+
+        // Check that connector0 was opened OK
+        assertThat(opened.get(),greaterThan(0));
+        // and it is now closed
+        assertFalse(connector.isOpen());
+        assertFalse(connector.isRunning());
+
+        // Check that server components were stopped
+        assertFalse(server.getBean(QueuedThreadPool.class).isStarted());
+    }
+
 
     public static class HelloHandler extends AbstractHandler
     {