Issue #3202

Handle async cross context session completion

jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java

@@ -1611,7 +1611,7 @@ public class SessionHandler extends ScopedHandler
     @Override
     public void doScope(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
     {
-        SessionHandler old_session_manager = null;
+        SessionHandler old_session_handler = null;
         HttpSession old_session = null;
         HttpSession existingSession = null;
 
@@ -1620,10 +1620,10 @@ public class SessionHandler extends ScopedHandler
             if (LOG.isDebugEnabled())
                 LOG.debug("SessionHandler.doScope");
             
-            old_session_manager = baseRequest.getSessionHandler();
+            old_session_handler = baseRequest.getSessionHandler();
             old_session = baseRequest.getSession(false);
 
-            if (old_session_manager != this)
+            if (old_session_handler != this)
             {
                 // new session context
                 baseRequest.setSessionHandler(this);
@@ -1634,7 +1634,7 @@ public class SessionHandler extends ScopedHandler
             // access any existing session for this context
             existingSession = baseRequest.getSession(false);
             
-            if ((existingSession != null) && (old_session_manager != this))
+            if ((existingSession != null) && (old_session_handler != this))
             {
                 HttpCookie cookie = access(existingSession,request.isSecure());
                 // Handle changed ID or max-age refresh, but only if this is not a redispatched request
@@ -1656,13 +1656,17 @@ public class SessionHandler extends ScopedHandler
         {
             //if there is a session that was created during handling this context, then complete it
             if (LOG.isDebugEnabled())
-                LOG.debug("FinalSession={}, old_session_manager={}, this={}, calling complete={}", baseRequest.getSession(false), old_session_manager, this, (old_session_manager != this));
+                LOG.debug("FinalSession={}, old_session_handler={}, this={}, calling complete={}", baseRequest.getSession(false), old_session_handler, this, (old_session_handler != this));
-            if (old_session_manager != this)
+
+            // If we are leaving the scope of this session handler, ensure the session is completed
+            if (old_session_handler != this)
                 ensureCompletion(baseRequest);
-         
+
-            if (old_session_manager != null && old_session_manager != this)
+            // revert the session handler to the previous, unless it was null, in which case remember it as
+            // the first session handler encountered.
+            if (old_session_handler != null && old_session_handler != this)
             {
-                baseRequest.setSessionHandler(old_session_manager);
+                baseRequest.setSessionHandler(old_session_handler);
                 baseRequest.setSession(old_session);
             }
         }

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java

@@ -52,22 +52,20 @@ import org.junit.jupiter.api.Test;
  */
 public class AsyncTest
 {
-
     public static class LatchServlet extends HttpServlet
     {
-
         @Override
         protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
         {
             resp.getWriter().println("Latched");
         }
-        
     }
 
-
     @Test
     public void testSessionWithAsyncDispatch() throws Exception
     {
+        // Test async dispatch back to same context, which then creates a session.
+
         DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
         cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
         SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
@@ -118,6 +116,8 @@ public class AsyncTest
     @Test
     public void testSessionWithAsyncComplete() throws Exception
     {
+        // Test async write, which creates a session and completes outside of a dispatch
+
         DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
         cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
         SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
@@ -168,6 +168,9 @@ public class AsyncTest
     @Test
     public void testSessionWithCrossContextAsync() throws Exception
     {
+        // Test async dispatch from context A to context B then
+        // async dispatch back to context B, which then creates a session (in context B).
+
         DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
         cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
         SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
@@ -218,10 +221,13 @@ public class AsyncTest
         }
     }
 
-
     @Test
     public void testSessionWithCrossContextAsyncComplete() throws Exception
     {
+        // Test async dispatch from context A to context B, which then does an
+        // async write, which creates a session (in context A) and completes outside of a
+        // dispatch
+
         DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
         cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
         SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
@@ -240,7 +246,6 @@ public class AsyncTest
         ServletHolder latchHolder = new ServletHolder(latchServlet);
         contextB.addServlet(latchHolder, "/latch");
 
-
         server.start();
         int port = server.getPort();