From 06f3486b8dc9e8c5d5537d90ff63e8110ca093a5 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 4 Nov 2015 17:23:01 +1100 Subject: [PATCH] 481373 Corner cases where session may remain in JDBCSessionManager memory --- .../server/session/JDBCSessionIdManager.java | 91 +++++++++++++++---- .../server/session/JDBCSessionManager.java | 54 ++++++++++- 2 files changed, 128 insertions(+), 17 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java index 99108d86d7b..3bfb8ba2425 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java +++ b/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 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 expiredSessionIds, boolean forceDelete) + private void scavengeSessions (Set candidateIds, Set expiredSessionIds, boolean forceDelete) { Set remainingIds = new HashSet(expiredSessionIds); - Handler[] contexts = _server.getChildHandlersByClass(ContextHandler.class); - for (int i=0; contexts!=null && i managers = getAllSessionManagers(); + for (SessionManager m:managers) { - SessionHandler sessionHandler = ((ContextHandler)contexts[i]).getChildHandlerByClass(SessionHandler.class); - if (sessionHandler != null) + Set successfullyExpiredIds = ((JDBCSessionManager)m).expire(expiredSessionIds); + if (successfullyExpiredIds != null) { - SessionManager manager = sessionHandler.getSessionManager(); - if (manager != null && manager instanceof JDBCSessionManager) - { - Set successfullyExpiredIds = ((JDBCSessionManager)manager).expire(expiredSessionIds); - if (successfullyExpiredIds != null) - remainingIds.removeAll(successfullyExpiredIds); - } + remainingIds.removeAll(successfullyExpiredIds); + candidateIds.removeAll(successfullyExpiredIds); } } + //Any remaining ids are of those sessions that no context removed if (!remainingIds.isEmpty() && forceDelete) @@ -1402,6 +1404,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 candidateIds) + { + if (candidateIds.isEmpty()) + return; + + + Set 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 getAllCandidateExpiredSessionIds() + { + HashSet candidateIds = new HashSet<>(); + + Set managers = getAllSessionManagers(); + + for (SessionManager m:managers) + { + candidateIds.addAll(((JDBCSessionManager)m).getCandidateExpiredIds()); + } + + return candidateIds; + } + + + private Set getAllSessionManagers() + { + HashSet managers = new HashSet<>(); + + Handler[] contexts = _server.getChildHandlersByClass(ContextHandler.class); + for (int i=0; contexts!=null && i candidateIds) + { + Iterator 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 getCandidateExpiredIds () + { + HashSet expiredIds = new HashSet<>(); + + Iterator 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 * @param id the id