Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embedded Tomcat does not honor ServletContainerInitializers #321

Closed
nebhale opened this issue Feb 6, 2014 · 22 comments
Closed

Embedded Tomcat does not honor ServletContainerInitializers #321

nebhale opened this issue Feb 6, 2014 · 22 comments

Comments

@nebhale
Copy link
Member

@nebhale nebhale commented Feb 6, 2014

Currently, the embedded Tomcat container does not detect and execute ServletContainerInitializers. This means that dependencies that use them are never called and therefore are unable to contribute to the running application.

The embedded Tomcat container should be enhanced such that it properly notices the ServletContainerInitializers on the Application's classpath and executes them.

@philwebb

This comment has been minimized.

Copy link
Member

@philwebb philwebb commented Feb 6, 2014

This was actually an intentional design decision. The search algorithm used by the containers was problematic. It also causes problems when you want to develop an executable WAR as you often want a javax.servlet.ServletContainerInitializer for the WAR that is not executed when you run java -jar.

See the org.springframework.boot.context.embedded.ServletContextInitializer for an option that works with Spring Beans.

Do you have a specific case where this is causing problems or was it more of an observation?

@philwebb philwebb added the discussion label Feb 6, 2014
@nebhale

This comment has been minimized.

Copy link
Member Author

@nebhale nebhale commented Feb 7, 2014

The Cloud Foundry Java Buildpack contributes a JAR to do auto-reconfiguration (re-writing bean definitions, setting profiles, etc.). To get the auto-reconfiguration started we use a ServletContainerInitializer to set a ServletContext init-param.

I'm a little fuzzy on exactly how the ServletContextInitializer is supposed to work. Is it automatically detected in 3rd party JARs or does it have to be declared as a bean in an application's Context?

@philwebb

This comment has been minimized.

Copy link
Member

@philwebb philwebb commented Feb 7, 2014

The org.springframework.boot.context.embedded.ServletContextInitializer currently needs to be a bean. You could possibly use a ApplicationContextInitializer via SpringFactoriesLoader to hook in the contributions.

@nebhale

This comment has been minimized.

Copy link
Member Author

@nebhale nebhale commented Feb 10, 2014

I had a crack at the SpringFactoriesLoader this morning. I'm not claiming that I got it perfectly right, but my first impression is that it'll only work for JARs that are in the lib/ directory. Specifically, it seems that JARs passed in via -cp aren't scanned. Is that right? Would I need to make sure that any dependency I want scanned is located in the lib directory?

@philwebb

This comment has been minimized.

Copy link
Member

@philwebb philwebb commented Feb 11, 2014

Not sure I follow. Are you saying that SpringFactoriesLoader doesn't pick up META-INF/spring.factories files for jars passed in via -cp?

@nebhale

This comment has been minimized.

Copy link
Member Author

@nebhale nebhale commented Feb 12, 2014

That's my initial indication. I've put in on the back burner for a moment while I clear out some other stuff in our backlog. Leave this issue with me for a couple of days and I'll get back to you with a definitive answer.

@nebhale

This comment has been minimized.

Copy link
Member Author

@nebhale nebhale commented Mar 7, 2014

@philwebb I've finally gotten back to this particular issue and yes, I believe that SpringFactoriesLoader doesn't pick up META-INF/spring.factories files for jars passed in via -cp? I don't have a "simple" test-case, but I do have an example where it isn't working. I'd be happy to walk you through it or provide a (non-miminal) JAR to demonstrate.

@philwebb

This comment has been minimized.

Copy link
Member

@philwebb philwebb commented Mar 7, 2014

Unless @dsyer has time to look at this we might need to leave it as a known issue for 1.0.0. I need to focus the next few weeks on documentation and critical bugs.

Can we pick it up again when the pressure is off?

@dsyer

This comment has been minimized.

Copy link
Member

@dsyer dsyer commented Mar 7, 2014

I don't think we'll ever be able to magically execute all SCIs. That's why I left it as "discussion". And there's definitely nothing happening to that in 1.0. We can take the SpringFactoriesLoader question to another issue maybe (seems like it's more of a Spring problem than a Boot problem, but maybe we are doing something in our classloaders that's messing it up).

@nebhale

This comment has been minimized.

Copy link
Member Author

@nebhale nebhale commented Mar 8, 2014

Yeah, no worries. I left it until much too late for you guys, and am happy to let it go. I'll open another issue with the SpringFactoriesLoader stuff as well.

@felixbarny

This comment has been minimized.

Copy link

@felixbarny felixbarny commented May 31, 2015

@philwebb could you be a bit more explicit what lead you to this design decision? What's problematic about the search algorithm? Where in the boot source code do you disable that feature? Why wouldn't you want the ServletContainerInitializer to be executed when you run java -jar? Would you consider supporting ServletContainerInitializer if some of the issues with it could be resolved?

Also it's not magical to execute all ServletContainerInitializers. java.util.ServiceLoader.load(javax.servlet.ServletContainerInitializer) should do the trick as all implementations of that interface have to be listed in /META-INF/services/javax.servlet.ServletContainerInitializer.

Servlet 3.0 made a really good job at defining this interface and the @Web* annotations as it makes it possible to write modular extensions to web applications (like stagemonitor) that just have to be on the classpath to be active and that are completely technology agnostic. It's really a pity that boot beaks the spec multiple times (neither ServletContainerInitializer nor @Web* annotations are supported) which makes these mechanisms useless because they don't work with boot, which is a (btw great) technology that just can't be ignored.

The goal of these servlet 3.0 mechanisms is to enable integration of standardized components without the user having to explicitly configure something. It's sad that boot makes that impossible :(

@felixbarny

This comment has been minimized.

Copy link

@felixbarny felixbarny commented Jun 8, 2015

friendly ping 😉

@dsyer

This comment has been minimized.

Copy link
Member

@dsyer dsyer commented Jun 8, 2015

Equally friendly reminder that nothing has changed :-). And that you are commenting on a closed issue.

@felixbarny

This comment has been minimized.

Copy link

@felixbarny felixbarny commented Jun 8, 2015

I just want to know more about the reasons about this decision and wether you might overthink it. Did you read my comment from 8 days ago? It would be great if you could answer my questions.

@dsyer

This comment has been minimized.

Copy link
Member

@dsyer dsyer commented Jun 8, 2015

There are lifecycle issues. The servlet context is not available when it is needed from what I remember. I'd be happy to be proved wrong, so please feel free to try it and report back.

@felixbarny

This comment has been minimized.

Copy link

@felixbarny felixbarny commented Jun 8, 2015

Have you considered using org.apache.catalina.startup.ContextConfig as a LifecycleListener in org.springframework.boot.autoconfigure.websocket.TomcatWebSocketContainerCustomizer org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory? In ContextConfig#processServletContainerInitializers there is a ServiceLoader that loads all ServletContainerInitializers.

This is my quick and dirty try:

@Override
public void customize(ConfigurableEmbeddedServletContainer container) {
    final TomcatEmbeddedServletContainerFactory tomcatContainer = (TomcatEmbeddedServletContainerFactory) container;
    tomcatContainer.addContextLifecycleListeners(new ContextConfig());
}

Is that a route you are willing to go? Would you consider using ContextConfig as a default LifecycleListener?

@dsyer

This comment has been minimized.

Copy link
Member

@dsyer dsyer commented Jun 8, 2015

What does it have to do with web sockets (other than the fact that Tomcat packages its ws support as an SCI)? Isn't that the wrong place to put a customizer? Anyway, I wouldn't rule anything out, but I suspect you will have issues with some SCIs (in particular the one in Spring). Did you try it?

@felixbarny

This comment has been minimized.

Copy link

@felixbarny felixbarny commented Jun 8, 2015

I'm sorry, I meant org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory.
I've tried with spring-boot-sample-web-ui. It's starting and working without problems so far.

@dsyer

This comment has been minimized.

Copy link
Member

@dsyer dsyer commented Jun 8, 2015

Try all the samples, and also try deploying a war file to an existing container. That should shake anything loose that we missed so far.

Of course this is only the Tomcat support and we'd probably have to find a way to do the same with the other platforms, but it's a start if it works.

@felixbarny

This comment has been minimized.

Copy link

@felixbarny felixbarny commented Jun 8, 2015

Is a way to do a automated integration test for the samples?

@dsyer

This comment has been minimized.

Copy link
Member

@dsyer dsyer commented Jun 8, 2015

Not with a war deployment (as far as I know). But "mvn test" is a good start.

felixbarny pushed a commit to felixbarny/spring-boot that referenced this issue Jun 8, 2015
@felixbarny

This comment has been minimized.

Copy link

@felixbarny felixbarny commented Jun 9, 2015

The spring-boot-samples tests are passing. Deploying the spring-boot-sample-web-jsp war to tomcat did also work. But the tests in org.springframework.boot.autoconfigure.security.oauth2.SpringSecurityOAuth2AutoConfigurationTestsare failing with the following exception:

java.util.concurrent.ExecutionException: org.apache.catalina.LifecycleException: Failed to start component [StandardEngine[Tomcat].StandardHost[localhost].StandardContext[]]
    at java.util.concurrent.FutureTask.report(FutureTask.java:122)
    at java.util.concurrent.FutureTask.get(FutureTask.java:188)
    at org.apache.catalina.core.ContainerBase.startInternal(ContainerBase.java:917)
    at org.apache.catalina.core.StandardHost.startInternal(StandardHost.java:871)
    at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1409)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1399)
    at java.util.concurrent.FutureTask.run(FutureTask.java:262)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
    at java.lang.Thread.run(Thread.java:745)
Caused by: org.apache.catalina.LifecycleException: Failed to start component [StandardEngine[Tomcat].StandardHost[localhost].StandardContext[]]
    at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:154)
    ... 6 more
Caused by: java.lang.IllegalStateException: No SpringApplication sources have been defined. Either override the configure method or add an @Configuration annotation
    at org.springframework.util.Assert.state(Assert.java:392)
    at org.springframework.boot.context.web.SpringBootServletInitializer.createRootApplicationContext(SpringBootServletInitializer.java:103)
    at org.springframework.boot.context.web.SpringBootServletInitializer.onStartup(SpringBootServletInitializer.java:68)
    at org.springframework.web.SpringServletContainerInitializer.onStartup(SpringServletContainerInitializer.java:175)
    at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5156)
    at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
    ... 6 more

The Javadoc of SpringBootServletInitializer states:

Note that a WebApplicationInitializer is only needed if you are building a war file and
deploying it. If you prefer to run an embedded container then you won't need this at
all.

So we probably have to find a way how to not invoke SpringBootServletInitializer in case of a java -jar deployment. Do you have an idea how to do that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.