481373 Corner cases where session may remain in JDBCSessionManager memory

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

@@ -1250,6 +1250,8 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
      */
     private void scavenge ()
     {
+        Set<String> candidateIds = getAllCandidateExpiredSessionIds();
+        
         Connection connection = null;
         try
         {
@@ -1283,7 +1285,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
                         }
                     }
                 }
-                scavengeSessions(expiredSessionIds, false);
+                scavengeSessions(candidateIds, expiredSessionIds, false);
 
 
                 //Pass 2: find sessions that have expired a while ago for which this node was their last manager
@@ -1306,7 +1308,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
                                 if (LOG.isDebugEnabled()) LOG.debug ("Found expired sessionId="+sessionId+" last managed by "+getWorkerName());
                             }
                         }
-                        scavengeSessions(expiredSessionIds, false);
+                        scavengeSessions(candidateIds, expiredSessionIds, false);
                     }
 
 
@@ -1329,9 +1331,13 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
                                 if (LOG.isDebugEnabled()) LOG.debug ("Found expired sessionId="+sessionId);
                             }
                         }
-                        scavengeSessions(expiredSessionIds, true);
+                        scavengeSessions(candidateIds, expiredSessionIds, true);
                     }
                 }
+                
+                //Tell session managers to check remaining sessions in memory that may have expired 
+                //but are no longer in the database
+                scavengeSessions(candidateIds);
             }
         }
         catch (Exception e)
@@ -1363,24 +1369,20 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
     /**
      * @param expiredSessionIds
      */
-    private void scavengeSessions (Set<String> expiredSessionIds, boolean forceDelete)
+    private void scavengeSessions (Set<String> candidateIds, Set<String> expiredSessionIds, boolean forceDelete)
     {       
         Set<String> remainingIds = new HashSet<String>(expiredSessionIds);
-        Handler[] contexts = _server.getChildHandlersByClass(ContextHandler.class);
+        Set<SessionManager> managers = getAllSessionManagers();
-        for (int i=0; contexts!=null && i<contexts.length; i++)
+        for (SessionManager m:managers)
         {
-            SessionHandler sessionHandler = ((ContextHandler)contexts[i]).getChildHandlerByClass(SessionHandler.class);
+            Set<String> successfullyExpiredIds = ((JDBCSessionManager)m).expire(expiredSessionIds);
-            if (sessionHandler != null)
-            {
-                SessionManager manager = sessionHandler.getSessionManager();
-                if (manager != null && manager instanceof JDBCSessionManager)
-                {
-                    Set<String> successfullyExpiredIds = ((JDBCSessionManager)manager).expire(expiredSessionIds);
             if (successfullyExpiredIds != null)
+            {
                 remainingIds.removeAll(successfullyExpiredIds);
+                candidateIds.removeAll(successfullyExpiredIds);
             }
         }
-        }
+    
 
         //Any remaining ids are of those sessions that no context removed
         if (!remainingIds.isEmpty() && forceDelete)
@@ -1403,6 +1405,63 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager
         }
     }
     
+    /**
+     * These are the session ids that the session managers thought had 
+     * expired, but were not expired in the database. This could be
+     * because the session is live on another node, or that the
+     * session no longer exists in the database because some other
+     * node removed it.
+     * @param candidateIds
+     */
+    private void scavengeSessions (Set<String> candidateIds)
+    {
+        if (candidateIds.isEmpty())
+            return;
+        
+        
+        Set<SessionManager> managers = getAllSessionManagers();
+        
+        for (SessionManager m:managers)
+        {
+            //tell the session managers to check the sessions that have expired in memory
+            //if they are no longer in the database, they should be removed
+            ((JDBCSessionManager)m).expireCandidates(candidateIds);
+        }
+    }
+    
+    private Set<String>  getAllCandidateExpiredSessionIds()
+    {
+        HashSet<String> candidateIds = new HashSet<>();
+        
+        Set<SessionManager> managers = getAllSessionManagers();
+        
+        for (SessionManager m:managers)
+        {
+            candidateIds.addAll(((JDBCSessionManager)m).getCandidateExpiredIds());
+        }
+        
+        return candidateIds;
+    }
+    
+    
+    private Set<SessionManager> getAllSessionManagers()
+    {
+        HashSet<SessionManager> managers = new HashSet<>();
+    
+        Handler[] contexts = _server.getChildHandlersByClass(ContextHandler.class);
+        for (int i=0; contexts!=null && i<contexts.length; i++)
+        {
+            SessionHandler sessionHandler = ((ContextHandler)contexts[i]).getChildHandlerByClass(SessionHandler.class);
+            if (sessionHandler != null)
+            {
+                SessionManager manager = sessionHandler.getSessionManager();
+                if (manager != null && manager instanceof JDBCSessionManager)
+                    managers.add(manager);
+            }
+        }
+        return managers;
+    }
+
 
    
     

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

@@ -569,6 +569,11 @@ public class JDBCSessionManager extends AbstractSessionManager
             }
             else
             {
+                if (memSession != null)
+                {
+                    //Session must have been removed from db by another node
+                    removeSession(memSession, true);
+                }
                 //No session in db with matching id and context path.
                 LOG.debug("getSession({}): No session in database matching id={}",idInCluster,idInCluster);
             }
@@ -866,6 +871,53 @@ public class JDBCSessionManager extends AbstractSessionManager
         }
     }
     
+    protected void expireCandidates (Set<String> candidateIds)
+    {
+        Iterator<String> itor = candidateIds.iterator();
+        long now = System.currentTimeMillis();
+        while (itor.hasNext())
+        {
+            String id = itor.next();
+
+            //check if expired in db
+            try
+            {
+                Session memSession = _sessions.get(id);
+                if (memSession == null)
+                {
+                    continue; //no longer in memory
+                }
+
+                Session s = loadSession(id,  canonicalize(_context.getContextPath()), getVirtualHost(_context));
+                if (s == null)
+                {
+                    //session no longer exists, can be safely expired
+                    memSession.timeout();
+                }
+            }
+            catch (Exception e)
+            {
+                LOG.warn("Error checking db for expiry for session {}", id);
+            }
+        }
+    }
+    
+    protected Set<String> getCandidateExpiredIds ()
+    {
+        HashSet<String> expiredIds = new HashSet<>();
+
+        Iterator<String> itor = _sessions.keySet().iterator();
+        while (itor.hasNext())
+        {
+            String id = itor.next();
+            //check to see if session should have expired
+            Session session = _sessions.get(id);
+            if (session._expiryTime > 0 &&  System.currentTimeMillis() > session._expiryTime)
+                expiredIds.add(id);           
+        }
+        return expiredIds;
+    }
+
 
     /**
      * Load a session from the database