From 0214fe4b8253f0daa3b8a721d4e9faab192cb462 Mon Sep 17 00:00:00 2001
From: Andy Wilkinson <awilkinson@pivotal.io>
Date: Tue, 1 Dec 2015 14:55:53 +0000
Subject: [PATCH] Remove inconsistent synchronization from
 EmbeddedWebApplicationContext

Previously, EmbeddedWebApplicationContext used synchronized, but did
not do so consistently. It also synchronized on this so its lock was
exposed outside of the class, creating a risk of deadlock if a caller
synchronized incorrectly. Furthermore, not all fields on the class
were sychronized so the class wasn't truly thread-safe.

This commit attempts to rectify some of the problems above. The use
of synchronized has been dropped in favour of using a volatile field
for the embedded servlet container. Whenever this field is accessed,
a local variable is used to "cache" the value thereby preventing a
change on another thread from causing unwanted behaviour such as an
NPE.

Closes gh-4593
---
 .../EmbeddedWebApplicationContext.java        | 35 +++++++++++--------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContext.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContext.java
index 418a3ddb9c8..af71aa06b8b 100644
--- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContext.java
+++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/EmbeddedWebApplicationContext.java
@@ -95,7 +95,7 @@ public class EmbeddedWebApplicationContext extends GenericWebApplicationContext
 	 */
 	public static final String DISPATCHER_SERVLET_NAME = ServletContextInitializerBeans.DISPATCHER_SERVLET_NAME;
 
-	private EmbeddedServletContainer embeddedServletContainer;
+	private volatile EmbeddedServletContainer embeddedServletContainer;
 
 	private ServletConfig servletConfig;
 
@@ -138,10 +138,10 @@ public class EmbeddedWebApplicationContext extends GenericWebApplicationContext
 	@Override
 	protected void finishRefresh() {
 		super.finishRefresh();
-		startEmbeddedServletContainer();
-		if (this.embeddedServletContainer != null) {
-			publishEvent(new EmbeddedServletContainerInitializedEvent(this,
-					this.embeddedServletContainer));
+		EmbeddedServletContainer localContainer = startEmbeddedServletContainer();
+		if (localContainer != null) {
+			publishEvent(
+					new EmbeddedServletContainerInitializedEvent(this, localContainer));
 		}
 	}
 
@@ -151,15 +151,17 @@ public class EmbeddedWebApplicationContext extends GenericWebApplicationContext
 		stopAndReleaseEmbeddedServletContainer();
 	}
 
-	private synchronized void createEmbeddedServletContainer() {
-		if (this.embeddedServletContainer == null && getServletContext() == null) {
+	private void createEmbeddedServletContainer() {
+		EmbeddedServletContainer localContainer = this.embeddedServletContainer;
+		ServletContext localServletContext = getServletContext();
+		if (localContainer == null && localServletContext == null) {
 			EmbeddedServletContainerFactory containerFactory = getEmbeddedServletContainerFactory();
 			this.embeddedServletContainer = containerFactory
 					.getEmbeddedServletContainer(getSelfInitializer());
 		}
-		else if (getServletContext() != null) {
+		else if (localServletContext != null) {
 			try {
-				getSelfInitializer().onStartup(getServletContext());
+				getSelfInitializer().onStartup(localServletContext);
 			}
 			catch (ServletException ex) {
 				throw new ApplicationContextException("Cannot initialize servlet context",
@@ -284,16 +286,19 @@ public class EmbeddedWebApplicationContext extends GenericWebApplicationContext
 		}
 	}
 
-	private void startEmbeddedServletContainer() {
-		if (this.embeddedServletContainer != null) {
-			this.embeddedServletContainer.start();
+	private EmbeddedServletContainer startEmbeddedServletContainer() {
+		EmbeddedServletContainer localContainer = this.embeddedServletContainer;
+		if (localContainer != null) {
+			localContainer.start();
 		}
+		return localContainer;
 	}
 
-	private synchronized void stopAndReleaseEmbeddedServletContainer() {
-		if (this.embeddedServletContainer != null) {
+	private void stopAndReleaseEmbeddedServletContainer() {
+		EmbeddedServletContainer localContainer = this.embeddedServletContainer;
+		if (localContainer != null) {
 			try {
-				this.embeddedServletContainer.stop();
+				localContainer.stop();
 				this.embeddedServletContainer = null;
 			}
 			catch (Exception ex) {