Respect Tomcat's failCtxIfServletStartFails flag

Ensure that if the user has set `failCtxIfServletStartFails` to `true`
using a `ContextCustomizers` any Servlet init exceptions stop the
application from running.

Closes gh-14448

Author: philwebb

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatEmbeddedContext.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2017 the original author or authors.
+ * Copyright 2012-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,10 +17,12 @@
 package org.springframework.boot.web.embedded.tomcat;
 
 import org.apache.catalina.Container;
+import org.apache.catalina.LifecycleException;
 import org.apache.catalina.Manager;
 import org.apache.catalina.core.StandardContext;
 import org.apache.catalina.session.ManagerBase;
 
+import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ReflectionUtils;
 
@@ -59,7 +61,7 @@ public void setManager(Manager manager) {
 		super.setManager(manager);
 	}
 
-	public void deferredLoadOnStartup() {
+	public void deferredLoadOnStartup() throws LifecycleException {
 		// Some older Servlet frameworks (e.g. Struts, BIRT) use the Thread context class
 		// loader to create servlet instances in this phase. If they do that and then try
 		// to initialize them later the class loader may have changed, so wrap the call to
@@ -70,15 +72,20 @@ public void deferredLoadOnStartup() {
 		if (classLoader != null) {
 			existingLoader = ClassUtils.overrideThreadContextClassLoader(classLoader);
 		}
-
-		if (this.overrideLoadOnStart) {
-			// Earlier versions of Tomcat used a version that returned void. If that
-			// version is used our overridden loadOnStart method won't have been called
-			// and the original will have already run.
-			super.loadOnStartup(findChildren());
+		try {
+			if (this.overrideLoadOnStart) {
+				// Earlier versions of Tomcat used a version that returned void. If that
+				// version is used our overridden loadOnStart method won't have been
+				// called and the original will have already run.
+				boolean started = super.loadOnStartup(findChildren());
+				Assert.state(started,
+						"Unable to start embedded tomcat context " + getName());
+			}
 		}
-		if (existingLoader != null) {
-			ClassUtils.overrideThreadContextClassLoader(existingLoader);
+		finally {
+			if (existingLoader != null) {
+				ClassUtils.overrideThreadContextClassLoader(existingLoader);
+			}
 		}
 	}
 

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/tomcat/TomcatWebServer.java

@@ -223,10 +223,15 @@ public void start() throws WebServerException {
 	}
 
 	private void checkThatConnectorsHaveStarted() {
+		checkConnectorHasStarted(this.tomcat.getConnector());
 		for (Connector connector : this.tomcat.getService().findConnectors()) {
-			if (LifecycleState.FAILED.equals(connector.getState())) {
-				throw new ConnectorStartFailedException(connector.getPort());
-			}
+			checkConnectorHasStarted(connector);
+		}
+	}
+
+	private void checkConnectorHasStarted(Connector connector) {
+		if (LifecycleState.FAILED.equals(connector.getState())) {
+			throw new ConnectorStartFailedException(connector.getPort());
 		}
 	}
 

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/embedded/tomcat/TomcatServletWebServerFactoryTests.java

@@ -30,6 +30,7 @@
 import javax.naming.InitialContext;
 import javax.naming.NamingException;
 import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
 
 import org.apache.catalina.Container;
 import org.apache.catalina.Context;
@@ -41,6 +42,7 @@
 import org.apache.catalina.Valve;
 import org.apache.catalina.connector.Connector;
 import org.apache.catalina.core.AprLifecycleListener;
+import org.apache.catalina.core.StandardContext;
 import org.apache.catalina.core.StandardWrapper;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.util.CharsetMapper;
@@ -427,6 +429,21 @@ public void customTomcatHttpOnlyCookie() {
 		assertThat(context.getUseHttpOnly()).isFalse();
 	}
 
+	@Test
+	public void exceptionThrownOnLoadFailureWhenFailCtxIfServletStartFailsIsTrue() {
+		TomcatServletWebServerFactory factory = getFactory();
+		factory.addContextCustomizers((context) -> {
+			if (context instanceof StandardContext) {
+				((StandardContext) context).setFailCtxIfServletStartFails(true);
+			}
+		});
+		this.webServer = factory.getWebServer((context) -> {
+			context.addServlet("failing", FailingServlet.class).setLoadOnStartup(0);
+		});
+		this.thrown.expect(WebServerException.class);
+		this.webServer.start();
+	}
+
 	@Override
 	protected JspServlet getJspServlet() throws ServletException {
 		Tomcat tomcat = ((TomcatWebServer) this.webServer).getTomcat();
@@ -475,4 +492,13 @@ protected void handleExceptionCausedByBlockedPort(RuntimeException ex,
 		assertThat(((ConnectorStartFailedException) ex).getPort()).isEqualTo(blockedPort);
 	}
 
+	static class FailingServlet extends HttpServlet {
+
+		@Override
+		public void init() throws ServletException {
+			throw new RuntimeException("Init Failure");
+		}
+
+	}
+
 }