Skip to content

Commit 0214fe4

Browse files
committed
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
1 parent 9bffdc8 commit 0214fe4

File tree

1 file changed

+20
-15
lines changed

1 file changed

+20
-15
lines changed

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

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public class EmbeddedWebApplicationContext extends GenericWebApplicationContext
9595
*/
9696
public static final String DISPATCHER_SERVLET_NAME = ServletContextInitializerBeans.DISPATCHER_SERVLET_NAME;
9797

98-
private EmbeddedServletContainer embeddedServletContainer;
98+
private volatile EmbeddedServletContainer embeddedServletContainer;
9999

100100
private ServletConfig servletConfig;
101101

@@ -138,10 +138,10 @@ protected void onRefresh() {
138138
@Override
139139
protected void finishRefresh() {
140140
super.finishRefresh();
141-
startEmbeddedServletContainer();
142-
if (this.embeddedServletContainer != null) {
143-
publishEvent(new EmbeddedServletContainerInitializedEvent(this,
144-
this.embeddedServletContainer));
141+
EmbeddedServletContainer localContainer = startEmbeddedServletContainer();
142+
if (localContainer != null) {
143+
publishEvent(
144+
new EmbeddedServletContainerInitializedEvent(this, localContainer));
145145
}
146146
}
147147

@@ -151,15 +151,17 @@ protected void onClose() {
151151
stopAndReleaseEmbeddedServletContainer();
152152
}
153153

154-
private synchronized void createEmbeddedServletContainer() {
155-
if (this.embeddedServletContainer == null && getServletContext() == null) {
154+
private void createEmbeddedServletContainer() {
155+
EmbeddedServletContainer localContainer = this.embeddedServletContainer;
156+
ServletContext localServletContext = getServletContext();
157+
if (localContainer == null && localServletContext == null) {
156158
EmbeddedServletContainerFactory containerFactory = getEmbeddedServletContainerFactory();
157159
this.embeddedServletContainer = containerFactory
158160
.getEmbeddedServletContainer(getSelfInitializer());
159161
}
160-
else if (getServletContext() != null) {
162+
else if (localServletContext != null) {
161163
try {
162-
getSelfInitializer().onStartup(getServletContext());
164+
getSelfInitializer().onStartup(localServletContext);
163165
}
164166
catch (ServletException ex) {
165167
throw new ApplicationContextException("Cannot initialize servlet context",
@@ -284,16 +286,19 @@ protected void prepareEmbeddedWebApplicationContext(ServletContext servletContex
284286
}
285287
}
286288

287-
private void startEmbeddedServletContainer() {
288-
if (this.embeddedServletContainer != null) {
289-
this.embeddedServletContainer.start();
289+
private EmbeddedServletContainer startEmbeddedServletContainer() {
290+
EmbeddedServletContainer localContainer = this.embeddedServletContainer;
291+
if (localContainer != null) {
292+
localContainer.start();
290293
}
294+
return localContainer;
291295
}
292296

293-
private synchronized void stopAndReleaseEmbeddedServletContainer() {
294-
if (this.embeddedServletContainer != null) {
297+
private void stopAndReleaseEmbeddedServletContainer() {
298+
EmbeddedServletContainer localContainer = this.embeddedServletContainer;
299+
if (localContainer != null) {
295300
try {
296-
this.embeddedServletContainer.stop();
301+
localContainer.stop();
297302
this.embeddedServletContainer = null;
298303
}
299304
catch (Exception ex) {

0 commit comments

Comments
 (0)