diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java index 764cc78a5f8..725aeb5f40d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java @@ -182,6 +182,14 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu __serverInfo = serverInfo; } + public enum ContextStatus + { + NOTSET, + INITIALIZED, + DESTROYED + } + + protected ContextStatus _contextStatus = ContextStatus.NOTSET; protected Context _scontext; private final AttributesMap _attributes; private final Map _initParams; @@ -828,6 +836,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu // defers the calling of super.doStart() startContext(); + + contextInitialized(); _availability = Availability.AVAILABLE; LOG.info("Started {}", this); @@ -886,49 +896,97 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu addEventListener(new ManagedAttributeListener(this, StringUtil.csvSplit(managedAttributes))); super.doStart(); + } + /** + * Call the ServletContextListeners contextInitialized methods. + * This can be called from a ServletHandler during the proper sequence + * of initializing filters, servlets and listeners. However, if there is + * no ServletHandler, the ContextHandler will call this method during + * doStart(). + * + * @throws Exception + */ + public void contextInitialized() throws Exception + { // Call context listeners - _destroyServletContextListeners.clear(); - if (!_servletContextListeners.isEmpty()) + switch (_contextStatus) { - ServletContextEvent event = new ServletContextEvent(_scontext); - for (ServletContextListener listener : _servletContextListeners) + case NOTSET: { - callContextInitialized(listener, event); - _destroyServletContextListeners.add(listener); + try + { + _destroyServletContextListeners.clear(); + if (!_servletContextListeners.isEmpty()) + { + ServletContextEvent event = new ServletContextEvent(_scontext); + for (ServletContextListener listener : _servletContextListeners) + { + callContextInitialized(listener, event); + _destroyServletContextListeners.add(listener); + } + } + } + finally + { + _contextStatus = ContextStatus.INITIALIZED; + } + break; } + default: + break; + } + } + + /** + * Call the ServletContextListeners with contextDestroyed. + * This method can be called from a ServletHandler in the + * proper sequence of destroying filters, servlets and listeners. + * If there is no ServletHandler, the ContextHandler must ensure + * these listeners are called instead. + * + * @throws Exception + */ + public void contextDestroyed() throws Exception + { + switch (_contextStatus) + { + case INITIALIZED: + { + try + { + //Call context listeners + MultiException ex = new MultiException(); + ServletContextEvent event = new ServletContextEvent(_scontext); + Collections.reverse(_destroyServletContextListeners); + for (ServletContextListener listener : _destroyServletContextListeners) + { + try + { + callContextDestroyed(listener, event); + } + catch (Exception x) + { + ex.add(x); + } + } + ex.ifExceptionThrow(); + } + finally + { + _contextStatus = ContextStatus.DESTROYED; + } + break; + } + default: + break; } } protected void stopContext() throws Exception { - // Call the context listeners - ServletContextEvent event = new ServletContextEvent(_scontext); - Collections.reverse(_destroyServletContextListeners); - MultiException ex = new MultiException(); - for (ServletContextListener listener : _destroyServletContextListeners) - { - try - { - callContextDestroyed(listener, event); - } - catch (Exception x) - { - ex.add(x); - } - } - // stop all the handler hierarchy - try - { - super.doStop(); - } - catch (Exception x) - { - ex.add(x); - } - - ex.ifExceptionThrow(); + super.doStop(); } protected void callContextInitialized(ServletContextListener l, ServletContextEvent e) @@ -945,9 +1003,6 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu l.contextDestroyed(e); } - /* - * @see org.eclipse.thread.AbstractLifeCycle#doStop() - */ @Override protected void doStop() throws Exception { @@ -987,6 +1042,8 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu stopContext(); + contextDestroyed(); + // retain only durable listeners setEventListeners(_durableListeners.toArray(new EventListener[0])); _durableListeners.clear(); @@ -1019,6 +1076,7 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu } finally { + _contextStatus = ContextStatus.NOTSET; __context.set(oldContext); exitScope(null); LOG.info("Stopped {}", this); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java index cd10401fae3..dbc2973e3a8 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ContextHandlerTest.java @@ -27,6 +27,9 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.List; + +import javax.servlet.ServletContextEvent; +import javax.servlet.ServletContextListener; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -445,6 +448,22 @@ public class ContextHandlerTest assertThat(connector.getResponse("GET /foo/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo'")); assertThat(connector.getResponse("GET /foo/bar/xxx HTTP/1.0\n\n"), Matchers.containsString("ctx='/foo/bar'")); } + + @Test + public void testContextInitializationDestruction() throws Exception + { + Server server = new Server(); + ContextHandlerCollection contexts = new ContextHandlerCollection(); + server.setHandler(contexts); + + ContextHandler noServlets = new ContextHandler(contexts, "/noservlets"); + TestServletContextListener listener = new TestServletContextListener(); + noServlets.addEventListener(listener); + server.start(); + assertEquals(1, listener.initialized); + server.stop(); + assertEquals(1, listener.destroyed); + } @Test public void testContextVirtualGetContext() throws Exception @@ -840,4 +859,22 @@ public class ContextHandlerTest writer.println("ctx='" + request.getContextPath() + "'"); } } + + private static class TestServletContextListener implements ServletContextListener + { + public int initialized = 0; + public int destroyed = 0; + + @Override + public void contextInitialized(ServletContextEvent sce) + { + initialized++; + } + + @Override + public void contextDestroyed(ServletContextEvent sce) + { + destroyed++; + } + } } diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index 4f51a5605b7..eebc610f0b3 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -33,6 +33,7 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ConcurrentMap; +import java.util.function.Consumer; import java.util.stream.Stream; import javax.servlet.DispatcherType; import javax.servlet.Filter; @@ -307,6 +308,9 @@ public class ServletHandler extends ScopedHandler _servlets = shs; ServletMapping[] sms = (ServletMapping[])LazyList.toArray(servletMappings, ServletMapping.class); _servletMappings = sms; + + if (_contextHandler != null) + _contextHandler.contextDestroyed(); //Retain only Listeners added via jetty apis (is Source.EMBEDDED) List listenerHolders = new ArrayList<>(); @@ -733,30 +737,40 @@ public class ServletHandler extends ScopedHandler public void initialize() throws Exception { - _initialized = true; - MultiException mx = new MultiException(); - Stream.concat(Stream.concat( - Arrays.stream(_filters), - Arrays.stream(_servlets).sorted()), - Arrays.stream(_listeners)) - .forEach(h -> + Consumer> c = h -> + { + try { - try + if (!h.isStarted()) { - if (!h.isStarted()) - { - h.start(); - h.initialize(); - } + h.start(); + h.initialize(); } - catch (Throwable e) - { - LOG.debug(Log.EXCEPTION, e); - mx.add(e); - } - }); + } + catch (Throwable e) + { + LOG.debug(Log.EXCEPTION, e); + mx.add(e); + } + }; + + //Start the listeners so we can call them + Arrays.stream(_listeners).forEach(c); + + //call listeners contextInitialized + if (_contextHandler != null) + _contextHandler.contextInitialized(); + + //Only set initialized true AFTER the listeners have been called + _initialized = true; + + //Start the filters then the servlets + Stream.concat( + Arrays.stream(_filters), + Arrays.stream(_servlets).sorted()) + .forEach(c); mx.ifExceptionThrow(); } diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java index 771c1931571..7b9a4e885e9 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletContextHandlerTest.java @@ -35,6 +35,7 @@ import javax.servlet.Filter; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; import javax.servlet.FilterRegistration; +import javax.servlet.GenericServlet; import javax.servlet.Servlet; import javax.servlet.ServletContainerInitializer; import javax.servlet.ServletContext; @@ -106,6 +107,75 @@ public class ServletContextHandlerTest private LocalConnector _connector; private static final AtomicInteger __testServlets = new AtomicInteger(); + private static int __initIndex = 0; + private static int __destroyIndex = 0; + + public class StopTestFilter implements Filter + { + int _initIndex; + int _destroyIndex; + + @Override + public void init(FilterConfig filterConfig) throws ServletException + { + _initIndex = __initIndex++; + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, + ServletException + { + } + + @Override + public void destroy() + { + _destroyIndex = __destroyIndex++; + } + } + + public class StopTestServlet extends GenericServlet + { + int _initIndex; + int _destroyIndex; + + @Override + public void destroy() + { + _destroyIndex = __destroyIndex++; + super.destroy(); + } + + @Override + public void init() throws ServletException + { + _initIndex = __initIndex++; + super.init(); + } + + @Override + public void service(ServletRequest req, ServletResponse res) throws ServletException, IOException + { + } + } + + public class StopTestListener implements ServletContextListener + { + int _initIndex; + int _destroyIndex; + + @Override + public void contextInitialized(ServletContextEvent sce) + { + _initIndex = __initIndex++; + } + + @Override + public void contextDestroyed(ServletContextEvent sce) + { + _destroyIndex = __destroyIndex++; + } + } public static class MySCI implements ServletContainerInitializer { @@ -405,6 +475,37 @@ public class ServletContextHandlerTest _server.join(); } + @Test + public void testDestroyOrder() throws Exception + { + ContextHandlerCollection contexts = new ContextHandlerCollection(); + _server.setHandler(contexts); + + ServletContextHandler root = new ServletContextHandler(contexts, "/", ServletContextHandler.SESSIONS); + ListenerHolder listenerHolder = new ListenerHolder(); + StopTestListener stopTestListener = new StopTestListener(); + listenerHolder.setListener(stopTestListener); + root.getServletHandler().addListener(listenerHolder); + ServletHolder servletHolder = new ServletHolder(); + StopTestServlet stopTestServlet = new StopTestServlet(); + servletHolder.setServlet(stopTestServlet); + root.addServlet(servletHolder, "/test"); + FilterHolder filterHolder = new FilterHolder(); + StopTestFilter stopTestFilter = new StopTestFilter(); + filterHolder.setFilter(stopTestFilter); + root.addFilter(filterHolder, "/*", EnumSet.of(DispatcherType.REQUEST)); + _server.start(); + _server.stop(); + + assertEquals(0, stopTestListener._initIndex); //listeners contextInitialized called first + assertEquals(1, stopTestFilter._initIndex); //filters init + assertEquals(2, stopTestServlet._initIndex); //servlets init + + assertEquals(0, stopTestFilter._destroyIndex); //filters destroyed first + assertEquals(1, stopTestServlet._destroyIndex); //servlets destroyed next + assertEquals(2, stopTestListener._destroyIndex); //listener contextDestroyed last + } + @Test public void testAddSessionListener() throws Exception { @@ -436,6 +537,41 @@ public class ServletContextHandlerTest assertTrue((Boolean)root.getServletContext().getAttribute("MyContextListener.contextInitialized")); } + @Test + public void testContextInitializationDestruction() throws Exception + { + Server server = new Server(); + ContextHandlerCollection contexts = new ContextHandlerCollection(); + server.setHandler(contexts); + + ServletContextHandler root = new ServletContextHandler(contexts, "/"); + class TestServletContextListener implements ServletContextListener + { + public int initialized = 0; + public int destroyed = 0; + + @Override + public void contextInitialized(ServletContextEvent sce) + { + initialized++; + } + + @Override + public void contextDestroyed(ServletContextEvent sce) + { + destroyed++; + } + } + + TestServletContextListener listener = new TestServletContextListener(); + root.addEventListener(listener); + server.start(); + server.stop(); + assertEquals(1, listener.initialized); + server.stop(); + assertEquals(1, listener.destroyed); + } + @Test public void testListenersFromContextListener() throws Exception { diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletLifeCycleTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletLifeCycleTest.java index f41c3bbbf93..4587572027f 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletLifeCycleTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/ServletLifeCycleTest.java @@ -60,8 +60,8 @@ public class ServletLifeCycleTest context.getObjectFactory().addDecorator(new TestDecorator()); ServletHandler sh = context.getServletHandler(); - sh.addListener(new ListenerHolder(TestListener.class)); - context.addEventListener(context.getServletContext().createListener(TestListener2.class)); + sh.addListener(new ListenerHolder(TestListener.class)); //added directly to ServletHandler + context.addEventListener(context.getServletContext().createListener(TestListener2.class));//create,decorate and add listener to context - no holder! sh.addFilterWithMapping(TestFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST)); sh.addFilterWithMapping(new FilterHolder(context.getServletContext().createFilter(TestFilter2.class)), "/*", EnumSet.of(DispatcherType.REQUEST)); @@ -110,8 +110,6 @@ public class ServletLifeCycleTest server.stop(); assertThat(events, Matchers.contains( - "contextDestroyed class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener", - "contextDestroyed class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener2", "destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter2", "Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter2", "destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestFilter", @@ -122,6 +120,8 @@ public class ServletLifeCycleTest "destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestServlet2", "Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestServlet", "destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestServlet", + "contextDestroyed class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener", + "contextDestroyed class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener2", "Destroy class org.eclipse.jetty.servlet.ServletLifeCycleTest$TestListener" ));