Skip to content

Commit 0069e41

Browse files
committed
Prevent closed context from being refreshed again by an HTTP request
Prior to this commit, EmbeddedWebApplicationContext would perform its close processing and then stop the embedded container. This could lead to the closed context's dispatcher servlet handling an HTTP request and refreshing the context again. This opened up two possibilities that we need to avoid: 1. Another HTTP request could be received by the dispatcher servlet while the context is still being refreshed. This could lead to the context being used before its refreshed. I believe this could be the cause of the current modification exception described in gh-3239 and SPR-13123. 2. It can lead to a race during shutdown as the shutdown hook's attempt to close the context races with the refresh initiated by the HTTP request. This is possible as the shutdown hook bypasses the sychronization on startupShutdownMonitor that would normally prevent refresh and close from occurring in parallel. This race can lead to a deadlock as described in gh-4130 Closes gh-4130
1 parent 7b6de54 commit 0069e41

File tree

3 files changed

+49
-1
lines changed

3 files changed

+49
-1
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,8 @@ protected void finishRefresh() {
146146

147147
@Override
148148
protected void doClose() {
149-
super.doClose();
150149
stopAndReleaseEmbeddedServletContainer();
150+
super.doClose();
151151
}
152152

153153
private synchronized void createEmbeddedServletContainer() {

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,14 @@
3636
import org.junit.rules.ExpectedException;
3737
import org.mockito.InOrder;
3838
import org.springframework.beans.MutablePropertyValues;
39+
import org.springframework.beans.factory.DisposableBean;
3940
import org.springframework.beans.factory.config.BeanDefinition;
4041
import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
4142
import org.springframework.beans.factory.config.ConstructorArgumentValues;
4243
import org.springframework.beans.factory.config.Scope;
44+
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
4345
import org.springframework.beans.factory.support.RootBeanDefinition;
46+
import org.springframework.boot.context.embedded.MockEmbeddedServletContainerFactory.MockEmbeddedServletContainer;
4447
import org.springframework.context.ApplicationContextException;
4548
import org.springframework.context.ApplicationListener;
4649
import org.springframework.context.support.AbstractApplicationContext;
@@ -54,6 +57,7 @@
5457

5558
import static org.hamcrest.Matchers.equalTo;
5659
import static org.hamcrest.Matchers.instanceOf;
60+
import static org.hamcrest.Matchers.is;
5761
import static org.hamcrest.Matchers.nullValue;
5862
import static org.hamcrest.Matchers.sameInstance;
5963
import static org.junit.Assert.assertEquals;
@@ -429,6 +433,22 @@ public void doesNotReplaceExistingScopes() throws Exception { // gh-2082
429433
sameInstance(scope));
430434
}
431435

436+
@Test
437+
public void containerIsStoppedBeforeContextIsClosed() {
438+
addEmbeddedServletContainerFactoryBean();
439+
this.context.registerBeanDefinition("shutdownOrderingValidator",
440+
BeanDefinitionBuilder.rootBeanDefinition(ShutdownOrderingValidator.class)
441+
.addConstructorArgReference("embeddedServletContainerFactory")
442+
.getBeanDefinition());
443+
this.context.refresh();
444+
ShutdownOrderingValidator validator = this.context
445+
.getBean(ShutdownOrderingValidator.class);
446+
this.context.close();
447+
assertThat(validator.destroyed, is(true));
448+
assertThat(validator.containerStoppedFirst, is(true));
449+
450+
}
451+
432452
private void addEmbeddedServletContainerFactoryBean() {
433453
this.context.registerBeanDefinition("embeddedServletContainerFactory",
434454
new RootBeanDefinition(MockEmbeddedServletContainerFactory.class));
@@ -478,4 +498,25 @@ public void doFilter(ServletRequest request, ServletResponse response,
478498

479499
}
480500

501+
protected static class ShutdownOrderingValidator implements DisposableBean {
502+
503+
private final MockEmbeddedServletContainer servletContainer;
504+
505+
private boolean destroyed = false;
506+
507+
private boolean containerStoppedFirst = false;
508+
509+
ShutdownOrderingValidator(
510+
MockEmbeddedServletContainerFactory servletContainerFactory) {
511+
this.servletContainer = servletContainerFactory.getContainer();
512+
}
513+
514+
@Override
515+
public void destroy() {
516+
this.destroyed = true;
517+
this.containerStoppedFirst = this.servletContainer.isStopped();
518+
}
519+
520+
}
521+
481522
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ public static class MockEmbeddedServletContainer implements EmbeddedServletConta
9090

9191
private final int port;
9292

93+
private boolean stopped = false;
94+
9395
public MockEmbeddedServletContainer(ServletContextInitializer[] initializers,
9496
int port) {
9597
this.initializers = initializers;
@@ -188,6 +190,11 @@ public void start() throws EmbeddedServletContainerException {
188190
public void stop() {
189191
this.servletContext = null;
190192
this.registeredServlets.clear();
193+
this.stopped = true;
194+
}
195+
196+
public boolean isStopped() {
197+
return this.stopped;
191198
}
192199

193200
public Servlet[] getServlets() {

0 commit comments

Comments
 (0)