Fix #5835 Durable Filters, Servlets and Listeners

For non durable ServletContextListeners ensure that initialized is called if they are added during starting and that destroyed is called.
jetty · Dec 22, 2020 · 1fd8caf · 1fd8caf
1 parent 6a1bcea
commit 1fd8caf
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 22 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
@@ -689,7 +689,7 @@ public void addEventListener(EventListener listener)
     {
         _eventListeners.add(listener);
 
-        if (!(isStarted() || isStarting()))
+        if (!isRunning())
         {
             _durableListeners.add(listener);
         }
@@ -702,7 +702,16 @@ public void addEventListener(EventListener listener)
         }
 
         if (listener instanceof ServletContextListener)
-            _servletContextListeners.add((ServletContextListener)listener);
+        {
+            ServletContextListener scl = (ServletContextListener)listener;
+            _servletContextListeners.add(scl);
+            if (_contextStatus == ContextStatus.INITIALIZED)
+            {
+                if (isStarting())
+                    scl.contextInitialized(new ServletContextEvent(_scontext));
+                _destroyServletContextListeners.add(scl);
+            }
+        }
 
         if (listener instanceof ServletContextAttributeListener)
             _servletContextAttributeListeners.add((ServletContextAttributeListener)listener);
@@ -982,31 +991,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
-                {
-                    _destroyServletContextListeners.clear();
-                    if (!_servletContextListeners.isEmpty())
-                    {
-                        ServletContextEvent event = new ServletContextEvent(_scontext);
-                        for (ServletContextListener listener : _servletContextListeners)
-                        {
-                            callContextInitialized(listener, event);
-                            _destroyServletContextListeners.add(listener);
-                        }
-                    }
-                }
-                finally
+                ServletContextEvent event = new ServletContextEvent(_scontext);
+                for (ServletContextListener listener : _servletContextListeners)
                 {
-                    _contextStatus = ContextStatus.INITIALIZED;
+                    callContextInitialized(listener, event);
+                    _destroyServletContextListeners.add(listener);
                 }
-                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
@@ -27,6 +27,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;
@@ -482,6 +483,69 @@ public void testContextInitializationDestruction() throws Exception
         assertEquals(1, listener.destroyed);
     }
 
+    @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();
+            }
+        });
+
+        server.start();
+        assertThat(initialized.get(), is(2));
+
+        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));
+    }
+
     @Test
     public void testContextVirtualGetContext() throws Exception
     {