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.

Sign up for GitHub

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

Jump to bottom

Programmatically registered ServletContextListener is not called in 9.4.14 #3137

Closed
wilkinsona opened this issue Nov 22, 2018 · 10 comments
Closed

Programmatically registered ServletContextListener is not called in 9.4.14 #3137

wilkinsona opened this issue Nov 22, 2018 · 10 comments
Assignees
@janbartel

Comments

@wilkinsona
Copy link
Contributor

There appears to be a regression in 9.4.14 that means that a programmatically registered ServletContextListener is no longer called. The problem can be reproduced using the
attached Maven project. It contains a single test.

It should pass with 9.4.13:

$ ./mvnw clean test -P9.4.13
[INFO] Scanning for projects...
[INFO] 
[INFO] -------< com.example:jetty-servlet-context-listener-regression >--------
[INFO] Building jetty-servlet-context-listener-regression 0.0.1-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ jetty-servlet-context-listener-regression ---
[INFO] 
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ jetty-servlet-context-listener-regression ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/src/main/resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ jetty-servlet-context-listener-regression ---
[INFO] No sources to compile
[INFO] 
[INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ jetty-servlet-context-listener-regression ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/src/test/resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.1:testCompile (default-testCompile) @ jetty-servlet-context-listener-regression ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 1 source file to /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/target/test-classes
[INFO] 
[INFO] --- maven-surefire-plugin:2.12.4:test (default-test) @ jetty-servlet-context-listener-regression ---
[INFO] Surefire report directory: /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/target/surefire-reports

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.example.demo.JettyServletContextListenerTest
2018-11-22 17:21:03.502:INFO::main: Logging initialized @225ms to org.eclipse.jetty.util.log.StdErrLog
2018-11-22 17:21:03.565:INFO:oejs.Server:main: jetty-9.4.13.v20181111; built: 2018-11-12T02:04:36.039Z; git: 49123a3313e396028d0041f631b7243485755f63; jvm 1.8.0_181-b13
2018-11-22 17:21:03.652:INFO:oejw.StandardDescriptorProcessor:main: NO JSP Support for /, did not find org.eclipse.jetty.jsp.JettyJspServlet
2018-11-22 17:21:03.659:INFO:oejs.session:main: DefaultSessionIdManager workerName=node0
2018-11-22 17:21:03.659:INFO:oejs.session:main: No SessionScavenger set, using defaults
2018-11-22 17:21:03.661:INFO:oejs.session:main: node0 Scavenging every 600000ms
2018-11-22 17:21:03.680:INFO:oejsh.ContextHandler:main: Started o.e.j.w.WebAppContext@fad74ee{/,file:///Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/,AVAILABLE}
2018-11-22 17:21:03.681:INFO:oejs.Server:main: Started @404ms
2018-11-22 17:21:03.682:INFO:oejs.session:main: node0 Stopped scavenging
2018-11-22 17:21:03.683:INFO:oejsh.ContextHandler:main: Stopped o.e.j.w.WebAppContext@fad74ee{/,file:///Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/,UNAVAILABLE}
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.283 sec

Results :

Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.498 s
[INFO] Finished at: 2018-11-22T17:21:03Z
[INFO] ------------------------------------------------------------------------

And fail with 9.4.14:

$ ./mvnw clean test -P9.4.14
[INFO] Scanning for projects...
[INFO] 
[INFO] -------< com.example:jetty-servlet-context-listener-regression >--------
[INFO] Building jetty-servlet-context-listener-regression 0.0.1-SNAPSHOT
[INFO] --------------------------------[ jar ]---------------------------------
[INFO] 
[INFO] --- maven-clean-plugin:2.5:clean (default-clean) @ jetty-servlet-context-listener-regression ---
[INFO] Deleting /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/target
[INFO] 
[INFO] --- maven-resources-plugin:2.6:resources (default-resources) @ jetty-servlet-context-listener-regression ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/src/main/resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ jetty-servlet-context-listener-regression ---
[INFO] No sources to compile
[INFO] 
[INFO] --- maven-resources-plugin:2.6:testResources (default-testResources) @ jetty-servlet-context-listener-regression ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/src/test/resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.1:testCompile (default-testCompile) @ jetty-servlet-context-listener-regression ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 1 source file to /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/target/test-classes
[INFO] 
[INFO] --- maven-surefire-plugin:2.12.4:test (default-test) @ jetty-servlet-context-listener-regression ---
[INFO] Surefire report directory: /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/target/surefire-reports

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.example.demo.JettyServletContextListenerTest
2018-11-22 17:21:54.702:INFO::main: Logging initialized @238ms to org.eclipse.jetty.util.log.StdErrLog
2018-11-22 17:21:54.766:INFO:oejs.Server:main: jetty-9.4.14.v20181114; built: 2018-11-14T21:20:31.478Z; git: c4550056e785fb5665914545889f21dc136ad9e6; jvm 1.8.0_181-b13
2018-11-22 17:21:54.850:INFO:oejw.StandardDescriptorProcessor:main: NO JSP Support for /, did not find org.eclipse.jetty.jsp.JettyJspServlet
2018-11-22 17:21:54.858:INFO:oejs.session:main: DefaultSessionIdManager workerName=node0
2018-11-22 17:21:54.858:INFO:oejs.session:main: No SessionScavenger set, using defaults
2018-11-22 17:21:54.860:INFO:oejs.session:main: node0 Scavenging every 660000ms
2018-11-22 17:21:54.879:INFO:oejsh.ContextHandler:main: Started o.e.j.w.WebAppContext@fad74ee{/,file:///Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/,AVAILABLE}
2018-11-22 17:21:54.880:INFO:oejs.Server:main: Started @416ms
2018-11-22 17:21:54.881:INFO:oejs.session:main: node0 Stopped scavenging
2018-11-22 17:21:54.882:INFO:oejsh.ContextHandler:main: Stopped o.e.j.w.WebAppContext@fad74ee{/,file:///Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/,UNAVAILABLE}
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.286 sec <<< FAILURE!
servletContextListenerIsCalled(com.example.demo.JettyServletContextListenerTest)  Time elapsed: 0.252 sec  <<< FAILURE!
java.lang.AssertionError: 
Expected: is <true>
     but: was <false>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.junit.Assert.assertThat(Assert.java:956)
	at org.junit.Assert.assertThat(Assert.java:923)
	at com.example.demo.JettyServletContextListenerTest.servletContextListenerIsCalled(JettyServletContextListenerTest.java:38)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
	at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
	at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)


Results :

Failed tests:   servletContextListenerIsCalled(com.example.demo.JettyServletContextListenerTest): (..)

Tests run: 1, Failures: 1, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.573 s
[INFO] Finished at: 2018-11-22T17:21:54Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.12.4:test (default-test) on project jetty-servlet-context-listener-regression: There are test failures.
[ERROR] 
[ERROR] Please refer to /Users/awilkinson/dev/workspaces/spring-projects/spring-boot/1.5.x/jetty-servlet-context-listener-regression/target/surefire-reports for the individual test results.
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
@wilkinsona wilkinsona mentioned this issue Nov 22, 2018
Closed
@janbartel
Copy link
Contributor

@wilkinsona it was actually a bug that it was working in 9.4.13 (and probably before that too) :) There was an error in the ServletContextHandler.Context whereby when addListener(Listener) was called, it was incorrectly added directly to the listener list AND also added to the list of ListenerHolders. It should only have ever been added to the list of ListenerHolders. See commit 1845e6e.

All the ListenerHolders are converted to the actual Listener class before the webapp's beans are started, thus from 9.4.14 onwards its not possible to add a listener with a webapp bean.

Can you explain your use case and I'll try and suggest another way of achieving it?

@wilkinsona
Copy link
Contributor Author

Thanks for the background.

The attached Maven project is a minimal recreation of Spring Boot's embedded Jetty integration. We have a ServletContextInitializer contract that's called when the embedded container (Jetty, Tomcat, or Undertow) is starting. Implementations can then configure the servlet context by registering filters, servlets, and servlet context listeners. Currently, the servlet context initialiser's are called via an AbstractLifecycle subclass which is what I tried to represent in a minimal fashion in the attached project.

@janbartel
Copy link
Contributor

@wilkinsona so would it be possible to change to using a ServletContainerInitializer to do the additions of servlets/filters/listeners? You can set the context attribute "org.eclipse.jetty.containerInitializerOrder" to make sure your SCI is the first one called: use the name of the class (as returned by class.getName()), followed by "*" to match any other SCIs discovered. Eg "com.acme.FooInitializer, *".

Would that be possible, or does this have to take place external to the servlet api (and if so, why?)?

@wilkinsona
Copy link
Contributor Author

We deliberately do not support SCIs for the reasons explained here. If it's possible for us to register an SCI without also causing any other SCIs to be detected that might be an option, but a mechanism outside of the Servlet API like we've been using up until 9.4.14 would be preferable.

@janbartel
Copy link
Contributor

@wilkinsona ok, I have something else for you to try:

Instead of calling webappcontext.getServletContext().addListener(l), instead do webappcontext.addEventListener(l) instead: this will add the listener instance directly to the list of listeners and bypass the ListenerHolder etc.

As you are now effectively managing the lifecycle of that listener yourself, you should also really organize to remove it when the context stops. So you could modify the TestServletContextListener to pass in the WebAppContext, and in the contextDestroyed() method, you can remove itself from the list of listeners with webapp.removeEventListeners(this).

Let me know how that works out for you.

@wilkinsona
Copy link
Contributor Author

Thanks for the suggestion. Unfortunately, an important detail has been lost in my attempt to provide an example that's as minimal as possible.

There's a layer of indirection between the AbstractLifecyle sub-class and the code that calls addListener on the servlet context. That layer is Boot's ServletContextInitializer contract which may be implemented by user code. In Spring Boot's actual code, the AbstractLifecycle sub-class has one or more ServletContextInitializer implementations registered with it. When the server is starting up, it retrieves the ServletContext and then passes it to each of the ServletContextInitializer implementations in turn. These implementations are designed to be portable across all the embedded containers that we support so the contract needs to use ServletContext as the type that's passed in so we can't use the Jetty-specific addEventListener method instead. Perhaps there's a point earlier in the lifecycle where we can add listeners via the ServletContext before the holders are converted so that they're picked up?

Failing that, wasn't the bug in 9.4.13 that's fixed by 1845e6e a regression? It looks like it fixes #3097 which apparently worked fine in 9.4.12 and earlier. Would it be possible to restore 9.4.12's behaviour so that the scenario described in #3097 works and Spring Boot continues to work as it has since Jetty 8.1 (at least)?

@SpencerMalone SpencerMalone mentioned this issue Dec 9, 2018
Closed
3 tasks
@wilkinsona
Copy link
Contributor Author

Any news on this please? As things stand we're stuck on Jetty 9.4.12 which isn't viable in the long term.

@janbartel
Copy link
Contributor

@wilkinsona sorry for the delay, I've been travelling.

Here's a suggestion to try out: remove the ServletContextInitializerConfiguration.Initializer class and move the code from line 80 (https://github.com/spring-projects/spring-boot/blob/v2.0.6.RELEASE/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/ServletContextInitializerConfiguration.java#L80) up into the ServletContextInitializerConfiguration.configure() method at line 53 (https://github.com/spring-projects/spring-boot/blob/v2.0.6.RELEASE/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/embedded/jetty/ServletContextInitializerConfiguration.java#L53). I've done a quick test by hacking up a jetty Configuration that called webapp.getServletContext().addListener(foo) inside the Configuration.configure method and it seemed ok, but you'd need to test it in your environment to be sure.

@wilkinsona
Copy link
Contributor Author

Thanks very much, @janbartel. That appears to work nicely.

@janbartel
Copy link
Contributor

Problem solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@janbartel janbartel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@janbartel @wilkinsona