From 9a554af48092a6451d4b038b63f20c23bbedd6b6 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 19 Feb 2016 10:56:27 +0100 Subject: [PATCH] Ensure infinispan timed-out sessions handled correctly --- .../InfinispanSessionDataStore.java | 35 ++++++++--- .../InfinispanSessionIdManager.java | 62 +++++++++++++------ .../session/AbstractSessionIdManager.java | 5 ++ .../jetty/server/session/SessionData.java | 7 ++- .../jetty/server/session/SessionManager.java | 3 +- .../session/InfinispanTestSessionServer.java | 1 + 6 files changed, 83 insertions(+), 30 deletions(-) diff --git a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java index 61e82796605..e4f3f535d7a 100644 --- a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java +++ b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionDataStore.java @@ -41,7 +41,6 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore { private final static Logger LOG = Log.getLogger("org.eclipse.jetty.server.session"); - public static final int DEFAULT_IDLE_EXPIRY_MULTIPLE = 2; /** @@ -51,8 +50,7 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore private SessionIdManager _idMgr = null; - - private int _idleExpiryMultiple = DEFAULT_IDLE_EXPIRY_MULTIPLE; + private int _infinispanIdleTimeoutSec; /** @@ -139,21 +137,25 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore long now = System.currentTimeMillis(); Set expired = new HashSet(); - if (LOG.isDebugEnabled()) - LOG.debug("Getting expired sessions " + now); for (String candidate:candidates) { + if (LOG.isDebugEnabled()) + LOG.debug("Checking expiry for candidate {}", candidate); try { SessionData sd = load(candidate); - if (sd == null || sd.isExpiredAt(now)) + + if (sd == null || sd.isExpiredAt(now)) + { expired.add(candidate); - + if (LOG.isDebugEnabled()) + LOG.debug("Is null {} is expired {}", (sd==null), (sd !=null)); + } } catch (Exception e) { - LOG.warn("Error checking if session {} is expired", candidate, e); + LOG.warn("Error checking if candidate {} is expired", candidate, e); } } @@ -170,10 +172,13 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore //if no requests arrive at any node before this timeout occurs, or no node //scavenges the session before this timeout occurs, the session will be removed. //NOTE: that no session listeners can be called for this. - if (data.getMaxInactiveMs() > 0) - _cache.put(getCacheKey(id, _context), data, -1, TimeUnit.MILLISECONDS, (data.getMaxInactiveMs() * _idleExpiryMultiple), TimeUnit.MILLISECONDS); + if (data.getMaxInactiveMs() > 0 && getInfinispanIdleTimeoutSec() > 0) + _cache.put(getCacheKey(id, _context), data, -1, TimeUnit.MILLISECONDS, getInfinispanIdleTimeoutSec(), TimeUnit.SECONDS); else _cache.put(getCacheKey(id, _context), data); + + if (LOG.isDebugEnabled()) + LOG.debug("Session {} saved to infinispan, expires {} ", id, data.getExpiry()); //tickle the session id manager to keep the sessionid entry for this session up-to-date if (_idMgr != null && _idMgr instanceof InfinispanSessionIdManager) @@ -198,4 +203,14 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore { return true; } + + public void setInfinispanIdleTimeoutSec (int sec) + { + _infinispanIdleTimeoutSec = sec; + } + + public int getInfinispanIdleTimeoutSec () + { + return _infinispanIdleTimeoutSec; + } } diff --git a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionIdManager.java b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionIdManager.java index 2ef60dae8fa..3260f748981 100644 --- a/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionIdManager.java +++ b/jetty-infinispan/src/main/java/org/eclipse/jetty/session/infinispan/InfinispanSessionIdManager.java @@ -26,11 +26,12 @@ import javax.servlet.http.HttpSession; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.SessionManager; + import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.session.AbstractSessionIdManager; import org.eclipse.jetty.server.session.Session; import org.eclipse.jetty.server.session.SessionHandler; +import org.eclipse.jetty.server.session.SessionManager; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.infinispan.commons.api.BasicCache; @@ -68,9 +69,8 @@ public class InfinispanSessionIdManager extends AbstractSessionIdManager { private final static Logger LOG = Log.getLogger("org.eclipse.jetty.server.session"); public final static String ID_KEY = "__o.e.j.s.infinispanIdMgr__"; - public static final int DEFAULT_IDLE_EXPIRY_MULTIPLE = 2; protected BasicCache _cache; - private int _idleExpiryMultiple = DEFAULT_IDLE_EXPIRY_MULTIPLE; + private int _infinispanIdleTimeoutSec = 0; @@ -141,22 +141,27 @@ public class InfinispanSessionIdManager extends AbstractSessionIdManager LOG.warn("Problem checking inUse for id="+clusterId, e); return false; } - + + } + + + public void setInfinispanIdleTimeoutSec (int sec) + { + if (sec <= 1) + { + LOG.warn("Idle expiry multiple of {} for session ids set to less than minimum. Using value of {} instead.", sec, 0); + _infinispanIdleTimeoutSec = 0; + } + else + _infinispanIdleTimeoutSec = sec; } - public void setIdleExpiryMultiple (int multiplier) + + + public int getInfinispanIdleTimeoutSec() { - if (multiplier <= 1) - { - LOG.warn("Idle expiry multiple of {} for session ids set to less than minimum. Using value of {} instead.", multiplier, DEFAULT_IDLE_EXPIRY_MULTIPLE); - } - _idleExpiryMultiple = multiplier; - } - - public int getIdleExpiryMultiple () - { - return _idleExpiryMultiple; + return _infinispanIdleTimeoutSec; } @@ -273,10 +278,10 @@ public class InfinispanSessionIdManager extends AbstractSessionIdManager if (session == null) return; - if (session.getMaxInactiveInterval() == 0) - insert (session.getId()); + if (session.getMaxInactiveInterval() > 0 && getInfinispanIdleTimeoutSec() > 0) + insert (session.getId(), getInfinispanIdleTimeoutSec()); else - insert (session.getId(), session.getMaxInactiveInterval() * getIdleExpiryMultiple()); + insert (session.getId()); } /** @@ -287,4 +292,25 @@ public class InfinispanSessionIdManager extends AbstractSessionIdManager { return delete (id); } + + /* ------------------------------------------------------------ */ + /** + * Remove an id from use by telling all contexts to remove a session with this id. + * + * @see org.eclipse.jetty.server.SessionIdManager#expireAll(java.lang.String) + */ + @Override + public void expireAll(String id) + { + LOG.debug("Expiring "+id); + //take the id out of the list of known sessionids for this node - TODO consider if we didn't remove it from this node + //it is because infinispan probably already timed it out. So, we only want to expire it from memory and NOT load it if present + removeId(id); + //tell all contexts that may have a session object with this id to + //get rid of them + for (SessionManager manager:getSessionManagers()) + { + manager.invalidate(id); + } + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java index d8cca44a9e3..547585624ff 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionIdManager.java @@ -348,6 +348,9 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme @Override public void expireAll(String id) { + if (LOG.isDebugEnabled()) + LOG.debug("Expiring {}",id); + //take the id out of the list of known sessionids for this node if (removeId(id)) { @@ -358,6 +361,8 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme manager.invalidate(id); } } + else if (LOG.isDebugEnabled()) + LOG.debug("Not present in idmgr: {}", id); } /* ------------------------------------------------------------ */ diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java index fff45883046..5588e299f16 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java @@ -26,6 +26,9 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + /** * SessionData * @@ -36,7 +39,7 @@ import java.util.concurrent.ConcurrentHashMap; */ public class SessionData implements Serializable { - + private final static Logger LOG = Log.getLogger("org.eclipse.jetty.server.session"); private static final long serialVersionUID = 1L; @@ -285,6 +288,8 @@ public class SessionData implements Serializable public boolean isExpiredAt (long time) { + if (LOG.isDebugEnabled()) + LOG.debug("Testing expiry on session {}: Never expires? {} Is expired?{}", _id, (getExpiry()<= 0), (getExpiry() < time)); if (getExpiry() <= 0) return false; //never expires diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionManager.java index 6df6acd4dad..ea9683d920a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionManager.java @@ -581,7 +581,6 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je session.setSessionManager(this); session.setLastNode(_sessionIdManager.getWorkerName()); session.getSessionData().setExpiry(_dftMaxIdleSecs <= 0 ? 0 : (created + _dftMaxIdleSecs*1000L)); - if (request.isSecure()) session.setAttribute(Session.SESSION_CREATED_SECURE, Boolean.TRUE); @@ -822,6 +821,8 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je } } } + //TODO if session object is not known to this node, how to get rid of it if no other + //node knows about it? return session; } diff --git a/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/InfinispanTestSessionServer.java b/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/InfinispanTestSessionServer.java index be0516684f3..2a6252ee018 100644 --- a/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/InfinispanTestSessionServer.java +++ b/tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/InfinispanTestSessionServer.java @@ -55,6 +55,7 @@ public class InfinispanTestSessionServer extends AbstractTestServer InfinispanSessionIdManager idManager = new InfinispanSessionIdManager(getServer()); idManager.setWorkerName("w"+(__workers++)); idManager.setCache((BasicCache)config); + idManager.setInfinispanIdleTimeoutSec(0); return idManager; }