[bs-239] Race condition in embedded container contexts
I'll mark this as fixing the bug in tracker, but it might benefit from some re-working at some point. The basic idea is to use our old friend ContextRefreshedEvent to start the server listening for connections. Delaying and making public the start() method from the EmbeddedServletContainer doesn't help because you need the server to start in order to get a ServletContext for Spring. [Fixes #53538787]
This commit is contained in:
parent
519d81cfdb
commit
ffa70f7c7e
|
|
@ -31,6 +31,8 @@ import java.util.Set;
|
|||
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
import org.springframework.context.ApplicationListener;
|
||||
import org.springframework.context.event.ContextRefreshedEvent;
|
||||
import org.springframework.util.Assert;
|
||||
|
||||
/**
|
||||
|
|
@ -40,12 +42,13 @@ import org.springframework.util.Assert;
|
|||
* @author Dave Syer
|
||||
*/
|
||||
public abstract class AbstractEmbeddedServletContainerFactory implements
|
||||
ConfigurableEmbeddedServletContainerFactory {
|
||||
ConfigurableEmbeddedServletContainerFactory,
|
||||
ApplicationListener<ContextRefreshedEvent> {
|
||||
|
||||
private static final String[] COMMON_DOC_ROOTS = { "src/main/webapp", "public",
|
||||
"static" };
|
||||
|
||||
private final Log logger = LogFactory.getLog(getClass());
|
||||
protected final Log logger = LogFactory.getLog(getClass());
|
||||
|
||||
private String contextPath = "";
|
||||
|
||||
|
|
@ -94,6 +97,14 @@ public abstract class AbstractEmbeddedServletContainerFactory implements
|
|||
this.port = port;
|
||||
}
|
||||
|
||||
/**
|
||||
* Subclasses should use this event to tell the server to start listening for
|
||||
* connections.
|
||||
*/
|
||||
@Override
|
||||
public void onApplicationEvent(ContextRefreshedEvent event) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the context path for the embedded servlet container. The context should start
|
||||
* with a "/" character but not end with a "/" character. The default context path can
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@
|
|||
|
||||
package org.springframework.bootstrap.context.embedded.jetty;
|
||||
|
||||
import org.eclipse.jetty.server.Connector;
|
||||
import org.eclipse.jetty.server.Server;
|
||||
import org.springframework.bootstrap.context.embedded.EmbeddedServletContainer;
|
||||
import org.springframework.bootstrap.context.embedded.EmbeddedServletContainerException;
|
||||
|
|
@ -46,6 +47,13 @@ public class JettyEmbeddedServletContainer implements EmbeddedServletContainer {
|
|||
private synchronized void start() {
|
||||
try {
|
||||
this.server.start();
|
||||
// Start the server so the ServletContext is available, but stop the
|
||||
// connectors to prevent requests from being handled before the Spring context
|
||||
// is ready:
|
||||
Connector[] connectors = this.server.getConnectors();
|
||||
for (Connector connector : connectors) {
|
||||
connector.stop();
|
||||
}
|
||||
}
|
||||
catch (Exception ex) {
|
||||
throw new EmbeddedServletContainerException(
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ import java.util.Arrays;
|
|||
import java.util.Collection;
|
||||
import java.util.List;
|
||||
|
||||
import org.eclipse.jetty.server.Connector;
|
||||
import org.eclipse.jetty.server.Server;
|
||||
import org.eclipse.jetty.server.handler.ErrorHandler;
|
||||
import org.eclipse.jetty.servlet.ErrorPageErrorHandler;
|
||||
|
|
@ -34,10 +35,13 @@ import org.eclipse.jetty.webapp.Configuration;
|
|||
import org.eclipse.jetty.webapp.WebAppContext;
|
||||
import org.springframework.bootstrap.context.embedded.AbstractEmbeddedServletContainerFactory;
|
||||
import org.springframework.bootstrap.context.embedded.EmbeddedServletContainer;
|
||||
import org.springframework.bootstrap.context.embedded.EmbeddedServletContainerException;
|
||||
import org.springframework.bootstrap.context.embedded.EmbeddedServletContainerFactory;
|
||||
import org.springframework.bootstrap.context.embedded.ErrorPage;
|
||||
import org.springframework.bootstrap.context.embedded.ServletContextInitializer;
|
||||
import org.springframework.context.ApplicationListener;
|
||||
import org.springframework.context.ResourceLoaderAware;
|
||||
import org.springframework.context.event.ContextRefreshedEvent;
|
||||
import org.springframework.core.io.ResourceLoader;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.ClassUtils;
|
||||
|
|
@ -59,7 +63,8 @@ import org.springframework.util.StringUtils;
|
|||
* @see JettyEmbeddedServletContainer
|
||||
*/
|
||||
public class JettyEmbeddedServletContainerFactory extends
|
||||
AbstractEmbeddedServletContainerFactory implements ResourceLoaderAware {
|
||||
AbstractEmbeddedServletContainerFactory implements ResourceLoaderAware,
|
||||
ApplicationListener<ContextRefreshedEvent> {
|
||||
|
||||
private List<Configuration> configurations = new ArrayList<Configuration>();
|
||||
|
||||
|
|
@ -93,6 +98,22 @@ public class JettyEmbeddedServletContainerFactory extends
|
|||
super(contextPath, port);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onApplicationEvent(ContextRefreshedEvent event) {
|
||||
if (this.context != null && this.context.getServer() != null) {
|
||||
try {
|
||||
Connector[] connectors = this.context.getServer().getConnectors();
|
||||
for (Connector connector : connectors) {
|
||||
connector.start();
|
||||
}
|
||||
}
|
||||
catch (Exception ex) {
|
||||
throw new EmbeddedServletContainerException(
|
||||
"Unable to start embedded Jetty servlet container", ex);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public EmbeddedServletContainer getEmbeddedServletContainer(
|
||||
ServletContextInitializer... initializers) {
|
||||
|
|
|
|||
|
|
@ -28,6 +28,8 @@ import org.springframework.util.Assert;
|
|||
* {@link TomcatEmbeddedServletContainerFactory} and not directly.
|
||||
*
|
||||
* @author Phillip Webb
|
||||
* @author Dave Syer
|
||||
*
|
||||
* @see TomcatEmbeddedServletContainerFactory
|
||||
*/
|
||||
public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer {
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ import org.springframework.bootstrap.context.embedded.EmbeddedServletContainerFa
|
|||
import org.springframework.bootstrap.context.embedded.ErrorPage;
|
||||
import org.springframework.bootstrap.context.embedded.ServletContextInitializer;
|
||||
import org.springframework.context.ResourceLoaderAware;
|
||||
import org.springframework.context.event.ContextRefreshedEvent;
|
||||
import org.springframework.core.io.ResourceLoader;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.ClassUtils;
|
||||
|
|
@ -60,6 +61,7 @@ import org.springframework.util.ClassUtils;
|
|||
*
|
||||
* @author Phillip Webb
|
||||
* @author Dave Syer
|
||||
*
|
||||
* @see #setPort(int)
|
||||
* @see #setContextLifecycleListeners(Collection)
|
||||
* @see TomcatEmbeddedServletContainer
|
||||
|
|
@ -105,6 +107,19 @@ public class TomcatEmbeddedServletContainerFactory extends
|
|||
super(contextPath, port);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void onApplicationEvent(ContextRefreshedEvent event) {
|
||||
Connector connector = this.connector;
|
||||
if (connector != null) {
|
||||
try {
|
||||
connector.getProtocolHandler().resume();
|
||||
}
|
||||
catch (Exception e) {
|
||||
this.logger.error("Cannot start connector: ", e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public EmbeddedServletContainer getEmbeddedServletContainer(
|
||||
ServletContextInitializer... initializers) {
|
||||
|
|
@ -114,18 +129,28 @@ public class TomcatEmbeddedServletContainerFactory extends
|
|||
File baseDir = (this.baseDirectory != null ? this.baseDirectory
|
||||
: createTempDir("tomcat"));
|
||||
this.tomcat.setBaseDir(baseDir.getAbsolutePath());
|
||||
if (this.connector != null) {
|
||||
this.connector.setPort(getPort());
|
||||
this.tomcat.getService().addConnector(this.connector);
|
||||
this.tomcat.setConnector(this.connector);
|
||||
Connector connector = this.connector;
|
||||
if (connector != null) {
|
||||
connector.setPort(getPort());
|
||||
this.tomcat.getService().addConnector(connector);
|
||||
this.tomcat.setConnector(connector);
|
||||
}
|
||||
else {
|
||||
Connector connector = new Connector(
|
||||
"org.apache.coyote.http11.Http11NioProtocol");
|
||||
connector = new Connector("org.apache.coyote.http11.Http11NioProtocol");
|
||||
customizeConnector(connector);
|
||||
this.tomcat.getService().addConnector(connector);
|
||||
this.tomcat.setConnector(connector);
|
||||
}
|
||||
this.connector = connector;
|
||||
try {
|
||||
// Allow the server to start so the ServletContext is available, but stop the
|
||||
// connector to prevent requests from being handled before the Spring context
|
||||
// is ready:
|
||||
connector.getProtocolHandler().pause();
|
||||
}
|
||||
catch (Exception e) {
|
||||
this.logger.error("Cannot pause connector: ", e);
|
||||
}
|
||||
this.tomcat.getHost().setAutoDeploy(false);
|
||||
this.tomcat.getEngine().setBackgroundProcessorDelay(-1);
|
||||
|
||||
|
|
|
|||
|
|
@ -84,17 +84,18 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
|
|||
ConfigurableEmbeddedServletContainerFactory factory = getFactory();
|
||||
this.container = factory
|
||||
.getEmbeddedServletContainer(exampleServletRegistration());
|
||||
assertThat(getResponse("http://localhost:8080/hello"), equalTo("Hello World"));
|
||||
assertThat(getResponse(factory, "http://localhost:8080/hello"),
|
||||
equalTo("Hello World"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void emptyServer() throws Exception {
|
||||
public void emptyServerWhenPortIsZero() throws Exception {
|
||||
ConfigurableEmbeddedServletContainerFactory factory = getFactory();
|
||||
factory.setPort(0);
|
||||
this.container = factory
|
||||
.getEmbeddedServletContainer(exampleServletRegistration());
|
||||
this.thrown.expect(SocketException.class);
|
||||
getResponse("http://localhost:8080/hello");
|
||||
getResponse(factory, "http://localhost:8080/hello");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -102,9 +103,10 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
|
|||
ConfigurableEmbeddedServletContainerFactory factory = getFactory();
|
||||
this.container = factory
|
||||
.getEmbeddedServletContainer(exampleServletRegistration());
|
||||
start(factory);
|
||||
this.container.stop();
|
||||
this.thrown.expect(SocketException.class);
|
||||
getResponse("http://localhost:8080/hello");
|
||||
getResponse(null, "http://localhost:8080/hello");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -135,7 +137,8 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
|
|||
this.container = factory.getEmbeddedServletContainer(
|
||||
exampleServletRegistration(), new FilterRegistrationBean(
|
||||
new ExampleFilter()));
|
||||
assertThat(getResponse("http://localhost:8080/hello"), equalTo("[Hello World]"));
|
||||
assertThat(getResponse(factory, "http://localhost:8080/hello"),
|
||||
equalTo("[Hello World]"));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -165,7 +168,8 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
|
|||
factory.setPort(8081);
|
||||
this.container = factory
|
||||
.getEmbeddedServletContainer(exampleServletRegistration());
|
||||
assertThat(getResponse("http://localhost:8081/hello"), equalTo("Hello World"));
|
||||
assertThat(getResponse(factory, "http://localhost:8081/hello"),
|
||||
equalTo("Hello World"));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -174,7 +178,8 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
|
|||
factory.setContextPath("/say");
|
||||
this.container = factory
|
||||
.getEmbeddedServletContainer(exampleServletRegistration());
|
||||
assertThat(getResponse("http://localhost:8080/say/hello"), equalTo("Hello World"));
|
||||
assertThat(getResponse(factory, "http://localhost:8080/say/hello"),
|
||||
equalTo("Hello World"));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -204,6 +209,7 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
|
|||
ConfigurableEmbeddedServletContainerFactory factory = getFactory();
|
||||
this.container = factory
|
||||
.getEmbeddedServletContainer(exampleServletRegistration());
|
||||
start(factory);
|
||||
this.container.stop();
|
||||
this.container.stop();
|
||||
}
|
||||
|
|
@ -232,12 +238,15 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
|
|||
AbstractEmbeddedServletContainerFactory factory = getFactory();
|
||||
factory.setDocumentRoot(this.temporaryFolder.getRoot());
|
||||
this.container = factory.getEmbeddedServletContainer();
|
||||
assertThat(getResponse("http://localhost:8080/test.txt"), equalTo("test"));
|
||||
assertThat(getResponse(factory, "http://localhost:8080/test.txt"),
|
||||
equalTo("test"));
|
||||
}
|
||||
|
||||
// FIXME test error page
|
||||
|
||||
protected String getResponse(String url) throws IOException, URISyntaxException {
|
||||
protected String getResponse(EmbeddedServletContainerFactory factory, String url)
|
||||
throws IOException, URISyntaxException {
|
||||
start(factory);
|
||||
SimpleClientHttpRequestFactory clientHttpRequestFactory = new SimpleClientHttpRequestFactory();
|
||||
ClientHttpRequest request = clientHttpRequestFactory.createRequest(new URI(url),
|
||||
HttpMethod.GET);
|
||||
|
|
@ -250,6 +259,12 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
|
|||
}
|
||||
}
|
||||
|
||||
private void start(EmbeddedServletContainerFactory factory) {
|
||||
if (factory instanceof AbstractEmbeddedServletContainerFactory) {
|
||||
((AbstractEmbeddedServletContainerFactory) factory).onApplicationEvent(null);
|
||||
}
|
||||
}
|
||||
|
||||
protected abstract AbstractEmbeddedServletContainerFactory getFactory();
|
||||
|
||||
private ServletContextInitializer exampleServletRegistration() {
|
||||
|
|
|
|||
|
|
@ -203,6 +203,13 @@
|
|||
<artifactId>maven-source-plugin</artifactId>
|
||||
<version>2.2.1</version>
|
||||
</plugin>
|
||||
<plugin>
|
||||
<artifactId>exec-maven-plugin</artifactId>
|
||||
<version>1.2.1</version>
|
||||
<configuration>
|
||||
<mainClass>${start-class}</mainClass>
|
||||
</configuration>
|
||||
</plugin>
|
||||
<plugin>
|
||||
<artifactId>maven-surefire-plugin</artifactId>
|
||||
<version>2.15</version>
|
||||
|
|
|
|||
Loading…
Reference in New Issue