Issue #10942 Fix flakey IdleSessionTest

jetty-ee10/jetty-ee10-tests/jetty-ee10-test-sessions/jetty-ee10-test-sessions-common/src/main/java/org/eclipse/jetty/ee10/session/SessionTestSupport.java

@@ -16,6 +16,7 @@ package org.eclipse.jetty.ee10.session;
 import org.eclipse.jetty.ee10.servlet.ServletContextHandler;
 import org.eclipse.jetty.ee10.servlet.SessionHandler;
 import org.eclipse.jetty.ee10.webapp.WebAppContext;
+import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.NetworkConnector;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
@@ -96,6 +97,11 @@ public class SessionTestSupport
         return h;
     }
 
+    public void insertHandler(Handler.Singleton handler)
+    {
+        _server.insertHandler(handler);
+    }
+
     public ServerConnector getServerConnector()
     {
         return _server.getBean(ServerConnector.class);
@@ -104,7 +110,7 @@ public class SessionTestSupport
     public void start() throws Exception
     {
         // server -> contexts collection -> context handler -> session handler -> servlet handler
-        _server.setHandler(_contexts);
+         _server.getTail().setHandler(_contexts);
         _server.start();
     }
 

jetty-ee10/jetty-ee10-tests/jetty-ee10-test-sessions/jetty-ee10-test-sessions-common/src/test/java/org/eclipse/jetty/ee10/session/IdleSessionTest.java

@@ -14,6 +14,7 @@
 package org.eclipse.jetty.ee10.session;
 
 import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import jakarta.servlet.ServletException;
@@ -27,6 +28,9 @@ import org.eclipse.jetty.client.Request;
 import org.eclipse.jetty.ee10.servlet.ServletContextHandler;
 import org.eclipse.jetty.ee10.servlet.ServletHolder;
 import org.eclipse.jetty.logging.StacklessLogging;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.HttpStream;
+import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.server.Session;
 import org.eclipse.jetty.session.DefaultSessionCacheFactory;
 import org.eclipse.jetty.session.ManagedSession;
@@ -34,6 +38,7 @@ import org.eclipse.jetty.session.NullSessionCacheFactory;
 import org.eclipse.jetty.session.SessionDataStoreFactory;
 import org.eclipse.jetty.session.test.TestSessionDataStore;
 import org.eclipse.jetty.session.test.TestSessionDataStoreFactory;
+import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.thread.AutoLock;
 import org.junit.jupiter.api.Test;
 
@@ -50,10 +55,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  */
 public class IdleSessionTest
 {
-
-    protected TestServlet _servlet = new TestServlet();
-    protected SessionTestSupport _server1 = null;
-
     public void pause(int sec) throws InterruptedException
     {
         Thread.sleep(TimeUnit.SECONDS.toMillis(sec));
@@ -67,7 +68,7 @@ public class IdleSessionTest
     {
         String contextPath = "";
         String servletMapping = "/server";
-        int inactivePeriod = 10;
+        int inactivePeriod = 5;
         int scavengePeriod = 1;
         int evictionSec = 2; //evict from cache if idle for 2 sec
 
@@ -76,12 +77,12 @@ public class IdleSessionTest
         cacheFactory.setFlushOnResponseCommit(true);
         SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
 
-        _server1 = new SessionTestSupport(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        SessionTestSupport sessionTestSupport = new SessionTestSupport(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
-        ServletHolder holder = new ServletHolder(_servlet);
+        ServletHolder holder = new ServletHolder(new TestServlet());
-        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        ServletContextHandler contextHandler = sessionTestSupport.addContext(contextPath);
         contextHandler.addServlet(holder, servletMapping);
-        _server1.start();
+        sessionTestSupport.start();
-        int port1 = _server1.getPort();
+        int port1 = sessionTestSupport.getPort();
 
         try (StacklessLogging stackless = new StacklessLogging(IdleSessionTest.class.getPackage()))
         {
@@ -140,8 +141,8 @@ public class IdleSessionTest
             pause(evictionSec * 2);
 
             //stop the scavenger
-            if (_server1.getHouseKeeper() != null)
+            if (sessionTestSupport.getHouseKeeper() != null)
-                _server1.getHouseKeeper().stop();
+                sessionTestSupport.getHouseKeeper().stop();
 
             //check that the session is passivated
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
@@ -159,7 +160,7 @@ public class IdleSessionTest
         }
         finally
         {
-            _server1.stop();
+            sessionTestSupport.stop();
         }
     }
 
@@ -169,19 +170,21 @@ public class IdleSessionTest
         //test the NullSessionCache which does not support idle timeout
         String contextPath = "";
         String servletMapping = "/server";
-        int inactivePeriod = 20;
+        int inactivePeriod = 5;
-        int scavengePeriod = 3;
+        int scavengePeriod = 2;
 
         NullSessionCacheFactory cacheFactory = new NullSessionCacheFactory();
         cacheFactory.setFlushOnResponseCommit(true);
         SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
 
-        _server1 = new SessionTestSupport(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        SessionTestSupport sessionTestSupport = new SessionTestSupport(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
-        ServletHolder holder = new ServletHolder(_servlet);
+        ServletHolder holder = new ServletHolder(new TestServlet());
-        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        ServletContextHandler contextHandler = sessionTestSupport.addContext(contextPath);
         contextHandler.addServlet(holder, servletMapping);
-        _server1.start();
+        RequestListenerHandler listenerHandler = new RequestListenerHandler();
-        int port1 = _server1.getPort();
+        sessionTestSupport.insertHandler(listenerHandler);
+        sessionTestSupport.start();
+        int port1 = sessionTestSupport.getPort();
 
         try (StacklessLogging stackless = new StacklessLogging(IdleSessionTest.class.getPackage()))
         {
@@ -201,10 +204,13 @@ public class IdleSessionTest
             assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
 
             //make another request to reactivate the session
+            CountDownLatch countDownLatch = new CountDownLatch(1);
+            listenerHandler.setCountDownLatch(countDownLatch);
             Request request = client.newRequest(url + "?action=test");
             ContentResponse response2 = request.send();
             assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
-            
+            //ensure request fully finished
+            assertTrue(countDownLatch.await(5, TimeUnit.SECONDS));
             //check session still not in the cache
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
             assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
@@ -213,6 +219,7 @@ public class IdleSessionTest
             //deleting the sessions in the store
             ((TestSessionDataStore)contextHandler.getSessionHandler().getSessionCache().getSessionDataStore())._map.clear();
 
+            listenerHandler.setCountDownLatch(null);
             //make a request
             request = client.newRequest(url + "?action=testfail");
             response2 = request.send();
@@ -227,8 +234,8 @@ public class IdleSessionTest
             id = SessionTestSupport.extractSessionId(sessionCookie);
             
             //stop the scavenger
-            if (_server1.getHouseKeeper() != null)
+            if (sessionTestSupport.getHouseKeeper() != null)
-                _server1.getHouseKeeper().stop();
+                sessionTestSupport.getHouseKeeper().stop();
 
             //check that the session is passivated
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
@@ -247,7 +254,69 @@ public class IdleSessionTest
         }
         finally
         {
-            _server1.stop();
+            sessionTestSupport.stop();
+        }
+    }
+
+    public static class RequestListenerWrapper extends HttpStream.Wrapper
+    {
+        CountDownLatch _countDownLatch;
+
+        public RequestListenerWrapper(HttpStream wrapped, CountDownLatch countDownLatch)
+        {
+            super(wrapped);
+            _countDownLatch = countDownLatch;
+        }
+
+        /**
+         *
+         */
+        @Override
+        public void succeeded()
+        {
+            if (_countDownLatch != null)
+                _countDownLatch.countDown();
+            super.succeeded();
+        }
+
+        /**
+         * @param x the reason for the operation failure
+         */
+        @Override
+        public void failed(Throwable x)
+        {
+            if (_countDownLatch != null)
+                _countDownLatch.countDown();
+            super.failed(x);
+        }
+    }
+
+    public static class RequestListenerHandler extends Handler.Wrapper
+    {
+        CountDownLatch _countDownLatch;
+
+        /**
+         * @param request the HTTP request to handle
+         * @param response the HTTP response to handle
+         * @param callback the callback to complete when the handling is complete
+         * @return
+         * @throws Exception
+         */
+        @Override
+        public boolean handle(org.eclipse.jetty.server.Request request, Response response, Callback callback) throws Exception
+        {
+            request.addHttpStreamWrapper(s -> new RequestListenerWrapper(s, _countDownLatch));
+            return super.handle(request, response, callback);
+        }
+
+        public void setCountDownLatch(CountDownLatch countDownLatch)
+        {
+            _countDownLatch = countDownLatch;
+        }
+
+        public CountDownLatch getCountDownLatch()
+        {
+            return _countDownLatch;
         }
     }
 

jetty-ee9/jetty-ee9-tests/jetty-ee9-test-sessions/jetty-ee9-test-sessions-common/src/main/java/org/eclipse/jetty/ee9/session/SessionTestSupport.java

@@ -16,6 +16,7 @@ package org.eclipse.jetty.ee9.session;
 import org.eclipse.jetty.ee9.nested.SessionHandler;
 import org.eclipse.jetty.ee9.servlet.ServletContextHandler;
 import org.eclipse.jetty.ee9.webapp.WebAppContext;
+import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.NetworkConnector;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
@@ -95,6 +96,11 @@ public class SessionTestSupport
         return h;
     }
 
+    public void insertHandler(Handler.Singleton handler)
+    {
+        _server.insertHandler(handler);
+    }
+
     public ServerConnector getServerConnector()
     {
         return _server.getBean(ServerConnector.class);
@@ -103,7 +109,7 @@ public class SessionTestSupport
     public void start() throws Exception
     {
         // server -> contexts collection -> context handler -> session handler -> servlet handler
-        _server.setHandler(_contexts);
+        _server.getTail().setHandler(_contexts);
         _server.start();
     }
 

jetty-ee9/jetty-ee9-tests/jetty-ee9-test-sessions/jetty-ee9-test-sessions-common/src/test/java/org/eclipse/jetty/ee9/session/IdleSessionTest.java

@@ -14,6 +14,7 @@
 package org.eclipse.jetty.ee9.session;
 
 import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import jakarta.servlet.ServletException;
@@ -27,12 +28,16 @@ import org.eclipse.jetty.client.Request;
 import org.eclipse.jetty.ee9.servlet.ServletContextHandler;
 import org.eclipse.jetty.ee9.servlet.ServletHolder;
 import org.eclipse.jetty.logging.StacklessLogging;
+import org.eclipse.jetty.server.Handler;
+import org.eclipse.jetty.server.HttpStream;
+import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.session.DefaultSessionCacheFactory;
 import org.eclipse.jetty.session.ManagedSession;
 import org.eclipse.jetty.session.NullSessionCacheFactory;
 import org.eclipse.jetty.session.SessionDataStoreFactory;
 import org.eclipse.jetty.session.test.TestSessionDataStore;
 import org.eclipse.jetty.session.test.TestSessionDataStoreFactory;
+import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.thread.AutoLock;
 import org.junit.jupiter.api.Test;
 
@@ -49,10 +54,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  */
 public class IdleSessionTest
 {
-
-    protected TestServlet _servlet = new TestServlet();
-    protected SessionTestSupport _server1 = null;
-
     public void pause(int sec) throws InterruptedException
     {
         Thread.sleep(TimeUnit.SECONDS.toMillis(sec));
@@ -75,12 +76,12 @@ public class IdleSessionTest
         cacheFactory.setFlushOnResponseCommit(true);
         SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
 
-        _server1 = new SessionTestSupport(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        SessionTestSupport sessionTestSupport = new SessionTestSupport(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
-        ServletHolder holder = new ServletHolder(_servlet);
+        ServletHolder holder = new ServletHolder(new TestServlet());
-        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        ServletContextHandler contextHandler = sessionTestSupport.addContext(contextPath);
         contextHandler.addServlet(holder, servletMapping);
-        _server1.start();
+        sessionTestSupport.start();
-        int port1 = _server1.getPort();
+        int port1 = sessionTestSupport.getPort();
 
         try (StacklessLogging stackless = new StacklessLogging(IdleSessionTest.class.getPackage()))
         {
@@ -139,8 +140,8 @@ public class IdleSessionTest
             pause(evictionSec * 2);
 
             //stop the scavenger
-            if (_server1.getHouseKeeper() != null)
+            if (sessionTestSupport.getHouseKeeper() != null)
-                _server1.getHouseKeeper().stop();
+                sessionTestSupport.getHouseKeeper().stop();
 
             //check that the session is passivated
             assertFalse(contextHandler.getSessionHandler().getSessionManager().getSessionCache().contains(id));
@@ -158,7 +159,7 @@ public class IdleSessionTest
         }
         finally
         {
-            _server1.stop();
+            sessionTestSupport.stop();
         }
     }
 
@@ -175,12 +176,14 @@ public class IdleSessionTest
         cacheFactory.setFlushOnResponseCommit(true);
         SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
 
-        _server1 = new SessionTestSupport(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        SessionTestSupport sessionTestSupport = new SessionTestSupport(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
-        ServletHolder holder = new ServletHolder(_servlet);
+        ServletHolder holder = new ServletHolder(new TestServlet());
-        ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        ServletContextHandler contextHandler = sessionTestSupport.addContext(contextPath);
         contextHandler.addServlet(holder, servletMapping);
-        _server1.start();
+        RequestListenerHandler listenerHandler = new RequestListenerHandler();
-        int port1 = _server1.getPort();
+        sessionTestSupport.insertHandler(listenerHandler);
+        sessionTestSupport.start();
+        int port1 = sessionTestSupport.getPort();
 
         try (StacklessLogging stackless = new StacklessLogging(IdleSessionTest.class.getPackage()))
         {
@@ -200,10 +203,14 @@ public class IdleSessionTest
             assertTrue(contextHandler.getSessionHandler().getSessionManager().getSessionCache().getSessionDataStore().exists(id));
 
             //make another request to reactivate the session
+            CountDownLatch countDownLatch = new CountDownLatch(1);
+            listenerHandler.setCountDownLatch(countDownLatch);
             Request request = client.newRequest(url + "?action=test");
             ContentResponse response2 = request.send();
             assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
-            
+            //ensure request fully finished
+            assertTrue(countDownLatch.await(5, TimeUnit.SECONDS));
+
             //check session still not in the cache
             assertFalse(contextHandler.getSessionHandler().getSessionManager().getSessionCache().contains(id));
             assertTrue(contextHandler.getSessionHandler().getSessionManager().getSessionCache().getSessionDataStore().exists(id));
@@ -211,7 +218,9 @@ public class IdleSessionTest
             //While passivated, take some action to ensure that a reactivate won't work, like
             //deleting the sessions in the store
             ((TestSessionDataStore)contextHandler.getSessionHandler().getSessionManager().getSessionCache().getSessionDataStore())._map.clear();
+            assertTrue(((TestSessionDataStore)contextHandler.getSessionHandler().getSessionManager().getSessionCache().getSessionDataStore())._map.isEmpty());
 
+            listenerHandler.setCountDownLatch(null);
             //make a request
             request = client.newRequest(url + "?action=testfail");
             response2 = request.send();
@@ -226,8 +235,8 @@ public class IdleSessionTest
             id = SessionTestSupport.extractSessionId(sessionCookie);
             
             //stop the scavenger
-            if (_server1.getHouseKeeper() != null)
+            if (sessionTestSupport.getHouseKeeper() != null)
-                _server1.getHouseKeeper().stop();
+                sessionTestSupport.getHouseKeeper().stop();
 
             //check that the session is passivated
             assertFalse(contextHandler.getSessionHandler().getSessionManager().getSessionCache().contains(id));
@@ -246,7 +255,69 @@ public class IdleSessionTest
         }
         finally
         {
-            _server1.stop();
+            sessionTestSupport.stop();
+        }
+    }
+
+    public static class RequestListenerWrapper extends HttpStream.Wrapper
+    {
+        CountDownLatch _countDownLatch;
+        
+        public RequestListenerWrapper(HttpStream wrapped, CountDownLatch countDownLatch)
+        {
+            super(wrapped);
+            _countDownLatch = countDownLatch;
+        }
+
+        /**
+         * 
+         */
+        @Override
+        public void succeeded()
+        {
+            if (_countDownLatch != null)
+                _countDownLatch.countDown();
+            super.succeeded();
+        }
+
+        /**
+         * @param x the reason for the operation failure 
+         */
+        @Override
+        public void failed(Throwable x)
+        {
+            if (_countDownLatch != null)
+                _countDownLatch.countDown();
+            super.failed(x);
+        }
+    }
+    
+    public static class RequestListenerHandler extends Handler.Wrapper
+    {
+        CountDownLatch _countDownLatch;
+        
+        /**
+         * @param request the HTTP request to handle 
+         * @param response the HTTP response to handle
+         * @param callback the callback to complete when the handling is complete
+         * @return
+         * @throws Exception
+         */
+        @Override
+        public boolean handle(org.eclipse.jetty.server.Request request, Response response, Callback callback) throws Exception
+        {
+            request.addHttpStreamWrapper(s -> new RequestListenerWrapper(s, _countDownLatch));
+            return super.handle(request, response, callback);
+        }
+
+        public void setCountDownLatch(CountDownLatch countDownLatch)
+        {
+            _countDownLatch = countDownLatch;
+        }
+        
+        public CountDownLatch getCountDownLatch()
+        {
+            return _countDownLatch;
         }
     }