Enable JNDI lookups during app context refresh without changing TCCL

When Tomcat is starting up and JNDI is enabled, it binds the web app
class loader into its ContextBindings, thereby enabling JNDI lookups
on any thread that uses the web app class loader as its thread context
class loader. When Boot starts an application, the application context
is refreshed on the main thread which has the app class loader as its
TCCL. This meant that any JNDI lookups performed during refresh would
fail.

gh-2038 described this problem and a fix was made in ff99bb0. The
fix was to set the main thread's TCCL to be Tomcat's web app class
loader. This fixed the JNDI lookup problem, but it has become apparent
that it has caused other problems when testing an application.

The fix for gh-2038 sets the main thread's TCCL when embedded Tomcat
starts (during application context refresh) and then restores it when
embedded Tomcat stops (as a result of the application context being
closed). This causes problems during testing as, when application
context's are cached, the close is delayed. This means that the main
thread's TCCL isn't restored, causing subsequent tests to run with the
wrong TCCL.

This commit takes a different approach to fixing gh-2038. Rather than
changing the main thread's TCCL, it binds the app class loader into
Tomcat's ContextBindings, thereby enabling JNDI lookups from the main
thread. To avoid leaving a reference to the app class loader in
Tomcat's ContextBindings, it unbinds the app class loader at the end
of application context refresh. This narrows the scope of the fix so
that it only applies during application context refresh which is the
period in which JNDI lookups were problematic.

Note that the original fix could have been modified to restore the
TCCL once context refresh has completed rather than waiting for the
context to be closed. However, my feeling is that leaving the TCCL
unchanged and specifically addressing the JNDI problem by manipulating
the context bindings is a more precise, and hopefully safer,
solution.

Closes gh-6053

spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java

@@ -20,6 +20,8 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import javax.naming.NamingException;
+
 import org.apache.catalina.Container;
 import org.apache.catalina.Context;
 import org.apache.catalina.Engine;
@@ -30,6 +32,7 @@ import org.apache.catalina.connector.Connector;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.naming.ContextBindings;
 
 import org.springframework.boot.context.embedded.EmbeddedServletContainer;
 import org.springframework.boot.context.embedded.EmbeddedServletContainerException;
@@ -93,8 +96,14 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer
 			// We can re-throw failure exception directly in the main thread
 			rethrowDeferredStartupExceptions();
 
-			ClassLoader classLoader = findContext().getLoader().getClassLoader();
-			Thread.currentThread().setContextClassLoader(classLoader);
+			Context context = findContext();
+			try {
+				ContextBindings.bindClassLoader(context, context.getNamingToken(),
+						getClass().getClassLoader());
+			}
+			catch (NamingException ex) {
+				// Naming is not enabled. Continue
+			}
 
 			// Unlike Jetty, all Tomcat threads are daemon threads. We create a
 			// blocking non-daemon to stop immediate shutdown
@@ -180,6 +189,11 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer
 			throw new EmbeddedServletContainerException(
 					"Unable to start embedded Tomcat servlet container", ex);
 		}
+		finally {
+			Context context = findContext();
+			ContextBindings.unbindClassLoader(context, context.getNamingToken(),
+					getClass().getClassLoader());
+		}
 	}
 
 	private void checkThatConnectorsHaveStarted() {

spring-boot/src/test/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainerFactoryTests.java

@@ -24,6 +24,9 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
 
+import javax.naming.InitialContext;
+import javax.naming.NamingException;
+
 import org.apache.catalina.Container;
 import org.apache.catalina.Context;
 import org.apache.catalina.LifecycleEvent;
@@ -399,14 +402,34 @@ public class TomcatEmbeddedServletContainerFactoryTests
 	}
 
 	@Test
-	public void tcclOfMainThreadIsTomcatWebAppClassLoader() {
+	public void jndiLookupsCanBePerformedDuringApplicationContextRefresh()
+			throws NamingException {
 		Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
-		TomcatEmbeddedServletContainerFactory factory = getFactory();
+		TomcatEmbeddedServletContainerFactory factory = new TomcatEmbeddedServletContainerFactory(
+				0) {
+
+			@Override
+			protected TomcatEmbeddedServletContainer getTomcatEmbeddedServletContainer(
+					Tomcat tomcat) {
+				tomcat.enableNaming();
+				return super.getTomcatEmbeddedServletContainer(tomcat);
+			}
+
+		};
+
+		// Container is created in onRefresh
 		this.container = factory.getEmbeddedServletContainer();
+
+		// Lookups should now be possible
+		new InitialContext().lookup("java:comp/env");
+
+		// Called in finishRefresh, giving us an opportunity to remove the context binding
+		// and avoid a leak
 		this.container.start();
-		assertThat(Thread.currentThread().getContextClassLoader())
-				.isInstanceOf(TomcatEmbeddedWebappClassLoader.class);
-		this.container.stop();
+
+		// Lookups should no longer be possible
+		this.thrown.expect(NamingException.class);
+		new InitialContext().lookup("java:comp/env");
 	}
 
 	@Override