Fix #5835 Durable filters and servlets (#6027)

Fix #5835 Durable filters and servlets with a general ServletHandler cleanup update indexes after updating mapping update mappings/indexes before destroyed listeners Signed-off-by: Greg Wilkins <gregw@webtide.com>
jetty · Mar 22, 2021 · c59de80 · c59de80
1 parent cc81b30
commit c59de80
Show file tree

Hide file tree

Showing 7 changed files with 567 additions and 499 deletions.
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
@@ -651,7 +651,24 @@ public boolean addEventListener(EventListener listener)
             }
 
             if (listener instanceof ServletContextListener)
+            {
+                if (_contextStatus == ContextStatus.INITIALIZED)
+                {
+                    ServletContextListener scl = (ServletContextListener)listener;
+                    _destroyServletContextListeners.add(scl);
+                    if (isStarting())
+                    {
+                        LOG.warn("ContextListener {} added whilst starting {}", scl, this);
+                        callContextInitialized(scl, new ServletContextEvent(_scontext));
+                    }
+                    else
+                    {
+                        LOG.warn("ContextListener {} added after starting {}", scl, this);
+                    }
+                }
+
                 _servletContextListeners.add((ServletContextListener)listener);
+            }
 
             if (listener instanceof ServletContextAttributeListener)
                 _servletContextAttributeListeners.add((ServletContextAttributeListener)listener);
@@ -933,31 +950,19 @@ protected void startContext() throws Exception
     public void contextInitialized() throws Exception
     {
         // Call context listeners
-        switch (_contextStatus)
+        if (_contextStatus == ContextStatus.NOTSET)
         {
-            case NOTSET:
+            _contextStatus = ContextStatus.INITIALIZED;
+            _destroyServletContextListeners.clear();
+            if (!_servletContextListeners.isEmpty())
             {
-                try
+                ServletContextEvent event = new ServletContextEvent(_scontext);
+                for (ServletContextListener listener : _servletContextListeners)
                 {
-                    _destroyServletContextListeners.clear();
-                    if (!_servletContextListeners.isEmpty())
-                    {
-                        ServletContextEvent event = new ServletContextEvent(_scontext);
-                        for (ServletContextListener listener : _servletContextListeners)
-                        {
-                            callContextInitialized(listener, event);
-                            _destroyServletContextListeners.add(listener);
-                        }
-                    }
+                    callContextInitialized(listener, event);
+                    _destroyServletContextListeners.add(listener);
                 }
-                finally
-                {
-                    _contextStatus = ContextStatus.INITIALIZED;
-                }
-                break;
             }
-            default:
-                break;
         }
     }
 

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
@@ -22,6 +22,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 import javax.servlet.ServletContextEvent;
 import javax.servlet.ServletContextListener;
 import javax.servlet.http.HttpServletRequest;
@@ -39,6 +40,7 @@
 import org.hamcrest.Matchers;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.slf4j.LoggerFactory;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
@@ -778,6 +780,71 @@ public void testClassPathWithSpaces() throws IOException
         assertThat("classpath", classpath, containsString(jar.toString()));
     }
 
+    @Test
+    public void testNonDurableContextListener() throws Exception
+    {
+        Server server = new Server();
+        ContextHandler context = new ContextHandler();
+        server.setHandler(context);
+        AtomicInteger initialized = new AtomicInteger();
+        AtomicInteger destroyed = new AtomicInteger();
+
+        context.addEventListener(new ServletContextListener()
+        {
+            @Override
+            public void contextInitialized(ServletContextEvent sce)
+            {
+                initialized.incrementAndGet();
+                context.addEventListener(new ServletContextListener()
+                {
+                    @Override
+                    public void contextInitialized(ServletContextEvent sce)
+                    {
+                        initialized.incrementAndGet();
+                    }
+
+                    @Override
+                    public void contextDestroyed(ServletContextEvent sce)
+                    {
+                        destroyed.incrementAndGet();
+                    }
+                });
+            }
+
+            @Override
+            public void contextDestroyed(ServletContextEvent sce)
+            {
+                destroyed.incrementAndGet();
+            }
+        });
+
+        LoggerFactory.getLogger(ContextHandler.class).info("Expect WARN ContextListener ... add whilst starting ...");
+        server.start();
+        assertThat(initialized.get(), is(2));
+
+        LoggerFactory.getLogger(ContextHandler.class).info("Expect WARN ContextListener ... add after starting ...");
+        context.addEventListener(new ServletContextListener()
+        {
+            @Override
+            public void contextInitialized(ServletContextEvent sce)
+            {
+                // This should not get called because added after started
+                initialized.incrementAndGet();
+            }
+
+            @Override
+            public void contextDestroyed(ServletContextEvent sce)
+            {
+                destroyed.incrementAndGet();
+            }
+        });
+
+        assertThat(initialized.get(), is(2));
+
+        server.stop();
+        assertThat(destroyed.get(), is(3));
+    }
+
     private void checkResourcePathsForExampleWebApp(String root) throws IOException
     {
         File testDirectory = setupTestDirectory();