From b8d0ad50f86fe07f13f6bc8b7af8ead37797f0c7 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 29 Feb 2016 21:02:44 +0100 Subject: [PATCH] Reimplementation of expiry and introduction of passivating unused sessions. --- .../InfinispanSessionDataStore.java | 20 +- .../InfinispanSessionIdManager.java | 23 +- .../session/AbstractSessionIdManager.java | 40 ++-- .../server/session/AbstractSessionStore.java | 212 +++++++++++++++--- .../jetty/server/session/ExpiryInspector.java | 114 ++++++++++ .../server/session/FileSessionDataStore.java | 2 + .../jetty/server/session/IdleInspector.java | 108 +++++++++ .../server/session/JDBCSessionIdManager.java | 2 +- .../server/session/MemorySessionStore.java | 15 +- ...ger.java => PeriodicSessionInspector.java} | 59 ++--- .../eclipse/jetty/server/session/Session.java | 48 +++- .../jetty/server/session/SessionData.java | 44 +++- .../server/session/SessionInspector.java | 34 +++ .../jetty/server/session/SessionManager.java | 7 +- .../jetty/server/session/SessionStore.java | 8 +- .../server/session/SessionCookieTest.java | 9 +- .../session/AbstractImmortalSessionTest.java | 6 +- .../server/session/AbstractTestServer.java | 6 +- 18 files changed, 627 insertions(+), 130 deletions(-) create mode 100644 jetty-server/src/main/java/org/eclipse/jetty/server/session/ExpiryInspector.java create mode 100644 jetty-server/src/main/java/org/eclipse/jetty/server/session/IdleInspector.java rename jetty-server/src/main/java/org/eclipse/jetty/server/session/{SessionScavenger.java => PeriodicSessionInspector.java} (79%) create mode 100644 jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionInspector.java 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 e4f3f535d7a..5457845b854 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 @@ -97,6 +97,9 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore try { + if (LOG.isDebugEnabled()) + LOG.debug("Loading session {} from infinispan", id); + SessionData sd = (SessionData)_cache.get(getCacheKey(id, _context)); reference.set(sd); } @@ -201,9 +204,24 @@ public class InfinispanSessionDataStore extends AbstractSessionDataStore @Override public boolean isPassivating() { - return true; + //TODO run in the _context to ensure classloader is set + try + { + Class remoteClass = Thread.currentThread().getContextClassLoader().loadClass("org.infinispan.client.hotrod.RemoteCache"); + if (_cache.getClass().isAssignableFrom(remoteClass)) + { + return true; + } + return false; + } + catch (ClassNotFoundException e) + { + return false; + } } + + public void setInfinispanIdleTimeoutSec (int sec) { _infinispanIdleTimeoutSec = sec; 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 3260f748981..9f422167fc0 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 @@ -293,24 +293,7 @@ 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 f4e4aee5300..76dfda588a1 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 @@ -55,7 +55,7 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme protected String _workerAttr; protected long _reseed=100000L; protected Server _server; - protected SessionScavenger _scavenger; + protected PeriodicSessionInspector _scavenger; /* ------------------------------------------------------------ */ /** @@ -102,7 +102,7 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme /** * @param scavenger a SessionScavenger */ - public void setSessionScavenger (SessionScavenger scavenger) + public void setSessionScavenger (PeriodicSessionInspector scavenger) { _scavenger = scavenger; _scavenger.setSessionIdManager(this); @@ -285,7 +285,7 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme if (_scavenger == null) { LOG.warn("No SessionScavenger set, using defaults"); - _scavenger = new SessionScavenger(); + _scavenger = new PeriodicSessionInspector(); _scavenger.setSessionIdManager(this); } @@ -388,17 +388,14 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme //session data store, AND have listeners called on them. //BUT want to avoid loading into memory sessions that this node is not managing (eg have 3 nodes all running session mgrs, //all 3 find the expired session and load it into memory and expire it - if (removeId(id)) + removeId(id); + + //tell all contexts that may have a session object with this id to + //get rid of them + for (SessionManager manager:getSessionManagers()) { - //tell all contexts that may have a session object with this id to - //get rid of them - for (SessionManager manager:getSessionManagers()) - { - manager.invalidate(id); - } + manager.invalidate(id); } - else if (LOG.isDebugEnabled()) - LOG.debug("Not present in idmgr: {}", id); } /* ------------------------------------------------------------ */ @@ -408,17 +405,16 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme public void invalidateAll (String id) { //take the id out of the list of known sessionids for this node - if (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); - } - } + 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); + } } - + /* ------------------------------------------------------------ */ /** Generate a new id for a session and update across diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java index e5484d4634e..8176504628d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionStore.java @@ -20,10 +20,14 @@ package org.eclipse.jetty.server.session; import java.util.Collections; +import java.util.List; import java.util.Set; +import java.util.function.Consumer; +import java.util.stream.Stream; import javax.servlet.http.HttpServletRequest; +import org.eclipse.jetty.util.MultiException; import org.eclipse.jetty.util.component.AbstractLifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -43,8 +47,9 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements protected StalenessStrategy _staleStrategy; protected SessionManager _manager; protected SessionContext _context; - - + protected int _idlePassivationTimeoutSec; + private IdleInspector _idleInspector; + private ExpiryInspector _expiryInspector; /** @@ -91,16 +96,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements * @return true if removed false otherwise */ public abstract Session doDelete (String id); - - - - - /** - * Get a list of keys for sessions that the store thinks has expired - * @return ids of all Session objects that might have expired - */ - public abstract Set doGetExpiredCandidates(); - + @@ -128,7 +124,6 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements return _manager; } - /** * @see org.eclipse.jetty.server.session.SessionStore#initialize(org.eclipse.jetty.server.session.SessionContext) @@ -158,6 +153,8 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements _sessionDataStore.initialize(_context); _sessionDataStore.start(); + _expiryInspector = new ExpiryInspector(this, _manager.getSessionIdManager()); + super.doStart(); } @@ -168,6 +165,7 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements protected void doStop() throws Exception { _sessionDataStore.stop(); + _expiryInspector = null; super.doStop(); } @@ -204,6 +202,30 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements } + /** + * @see org.eclipse.jetty.server.session.SessionStore#getIdlePassivationTimeoutSec() + */ + public int getIdlePassivationTimeoutSec() + { + return _idlePassivationTimeoutSec; + } + + + + /** + * @see org.eclipse.jetty.server.session.SessionStore#setIdlePassivationTimeoutSec(int) + */ + public void setIdlePassivationTimeoutSec(int idleTimeoutSec) + { + _idlePassivationTimeoutSec = idleTimeoutSec; + if (_idlePassivationTimeoutSec == 0) + _idleInspector = null; + else if (_idleInspector == null) + _idleInspector = new IdleInspector(this); + } + + + /** * Get a session object. * @@ -219,28 +241,47 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements //look locally Session session = doGet(id); - - if (staleCheck && isStale(session)) - { - //delete from session store so should reload from session data store - doDelete(id); - session = null; - } - - //not in session store, load the data for the session if possible - if (session == null && _sessionDataStore != null) + //TODO also check that session is only written out if only the access time changes infrequently + + //session is either not in session store, or it is stale, or its been passivated, load the data for the session if possible + if (session == null || (staleCheck && isStale(session)) || session.isPassivated() && _sessionDataStore != null) { SessionData data = _sessionDataStore.load(id); - if (data != null) + + //session wasn't in session store + if (session == null) { - session = newSession(data); - session.setSessionManager(_manager); - Session existing = doPutIfAbsent(id, session); - if (existing != null) + if (data != null) { - //some other thread has got in first and added the session - //so use it - session = existing; + session = newSession(data); + session.setSessionManager(_manager); + Session existing = doPutIfAbsent(id, session); + if (existing != null) + { + //some other thread has got in first and added the session + //so use it + session = existing; + } + } + //else session not in store and not in data store either, so doesn't exist + } + else + { + //session was already in session store, refresh it if its still stale/passivated + try (Lock lock = session.lock()) + { + if (session.isPassivated() || staleCheck && isStale(session)) + { + //if we were able to load it, then update our session object + if (data != null) + { + session.setPassivated(false); + session.getSessionData().copy(data); + session.didActivate(); + } + else + session = null; //TODO rely on the expiry mechanism to get rid of it? + } } } } @@ -303,11 +344,39 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements /** * Remove a session object from this store and from any backing store. * + * If session has been passivated, may need to reload it before it can + * be properly deleted + * * @see org.eclipse.jetty.server.session.SessionStore#delete(java.lang.String) */ @Override public Session delete(String id) throws Exception { + //Ensure that the session object is not passivated so that its attributes + //are valid + Session session = doGet(id); + + //TODO if (session == null) do we want to load it to delete it? + if (session != null) + { + try (Lock lock = session.lock()) + { + //TODO don't check stale on deletion? + if (session.isPassivated() && _sessionDataStore != null) + { + session.setPassivated(false); + SessionData data = _sessionDataStore.load(id); + if (data != null) + { + session.getSessionData().copy(data); + session.didActivate(); + } + } + } + } + + + //Always delete it from the data store if (_sessionDataStore != null) { boolean dsdel = _sessionDataStore.delete(id); @@ -331,19 +400,93 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements + /** - * @see org.eclipse.jetty.server.session.SessionStore#getExpired() + * @see org.eclipse.jetty.server.session.SessionStore#checkExpiry(java.util.Set) */ @Override - public Set getExpired() + public Set checkExpiration(Set candidates) { if (!isStarted()) return Collections.emptySet(); - Set candidates = doGetExpiredCandidates(); + + if (LOG.isDebugEnabled()) + LOG.debug("SessionStore checking expiration on {}", candidates); return _sessionDataStore.getExpired(candidates); } + + + /** + * @see org.eclipse.jetty.server.session.SessionStore#inspect() + */ + public void inspect () + { + Stream stream = getStream(); + try + { + _expiryInspector.preInspection(); + if (_idleInspector != null) + _idleInspector.preInspection(); + stream.forEach(s->{_expiryInspector.inspect(s); if (_idleInspector != null) _idleInspector.inspect(s);}); + _expiryInspector.postInspection(); + _idleInspector.postInspection(); + } + finally + { + stream.close(); + } + } + + + + + /** + * If the SessionDataStore supports passivation, passivate any + * sessions that have not be accessed for longer than x sec + * + * @param id identity of session to passivate + */ + public void passivateIdleSession(String id) + { + if (!isStarted()) + return; + + if (_sessionDataStore == null) + return; //no data store to passivate + + if (!_sessionDataStore.isPassivating()) + return; //doesn't support passivation + + + //get the session locally + Session s = doGet(id); + + if (s == null) + { + LOG.warn("Session {} not in this session store", s); + return; + } + + + try (Lock lock = s.lock()) + { + //check the session is still idle first + if (s.isValid() && s.isIdleLongerThan(_idlePassivationTimeoutSec)) + { + s.willPassivate(); + _sessionDataStore.store(id, s.getSessionData()); + s.getSessionData().clearAllAttributes(); + s.getSessionData().setDirty(false); + } + } + catch (Exception e) + { + LOG.warn("Passivation of idle session {} failed", id, e); + // TODO should do session.invalidate(); ??? + } + } @@ -355,5 +498,4 @@ public abstract class AbstractSessionStore extends AbstractLifeCycle implements { return null; } - } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/ExpiryInspector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/ExpiryInspector.java new file mode 100644 index 00000000000..f7f80082c97 --- /dev/null +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/ExpiryInspector.java @@ -0,0 +1,114 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + + +package org.eclipse.jetty.server.session; + +import java.util.HashSet; +import java.util.Set; + +import org.eclipse.jetty.server.SessionIdManager; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + +/** + * ExpiryInspector + * + * + */ +public class ExpiryInspector implements SessionInspector +{ + private final static Logger LOG = Log.getLogger("org.eclipse.jetty.server.session"); + + protected Set _expiryCandidates; + protected SessionIdManager _idManager; + protected AbstractSessionStore _sessionStore; + + + + /** + * @param sessionStore + * @param idManager + */ + public ExpiryInspector (AbstractSessionStore sessionStore, SessionIdManager idManager) + { + _idManager = idManager; + _sessionStore = sessionStore; + } + + /** + * @see org.eclipse.jetty.server.session.SessionInspector#inspect(org.eclipse.jetty.server.session.SessionStore, org.eclipse.jetty.server.session.Session) + */ + @Override + public void inspect(Session s) + { + //Does the session object think it is expired? + long now = System.currentTimeMillis(); + if (s.isExpiredAt(now)) + _expiryCandidates.add(s.getId()); + } + + + + + /** + * @return the expiryCandidates + */ + public Set getExpiryCandidates() + { + return _expiryCandidates; + } + + /** + * @see org.eclipse.jetty.server.session.SessionInspector#preInspection() + */ + @Override + public void preInspection() + { + if (LOG.isDebugEnabled()) + LOG.debug("PreInspection"); + _expiryCandidates = new HashSet(); + } + + /** + * @see org.eclipse.jetty.server.session.SessionInspector#postInspection() + */ + @Override + public void postInspection() + { + if (LOG.isDebugEnabled()) + LOG.debug("ExpiryInspector checking expiration for {}", _expiryCandidates); + + try + { + Set candidates = _sessionStore.checkExpiration(_expiryCandidates); + for (String id:candidates) + { + _idManager.expireAll(id); + } + } + catch (Exception e) + { + LOG.warn(e); + } + finally + { + _expiryCandidates = null; + } + } +} diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java index 0686a5ddd5f..8ebb4291a3d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/FileSessionDataStore.java @@ -117,6 +117,8 @@ public class FileSessionDataStore extends AbstractSessionDataStore public Set getExpired(Set candidates) { //we don't want to open up each file and check, so just leave it up to the SessionStore + //TODO as the session manager is likely to be a lazy loader, if a session is never requested, its + //file will stay forever after a restart return candidates; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/IdleInspector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/IdleInspector.java new file mode 100644 index 00000000000..ca0d7979c02 --- /dev/null +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/IdleInspector.java @@ -0,0 +1,108 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + + +package org.eclipse.jetty.server.session; + +import java.util.HashSet; +import java.util.Set; + +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + + +/** + * IdleExpiryInspector + * + * Checks if a session is idle + */ +public class IdleInspector implements SessionInspector +{ + private final static Logger LOG = Log.getLogger("org.eclipse.jetty.server.session"); + + protected Set _idleCandidates; + protected AbstractSessionStore _sessionStore; + + /** + * @param sessionStore + */ + public IdleInspector (AbstractSessionStore sessionStore) + { + _sessionStore = sessionStore; + } + + + /** + * @see org.eclipse.jetty.server.session.SessionInspector#inspect(org.eclipse.jetty.server.session.Session) + */ + @Override + public void inspect(Session s) + { + //Does the session object think it is expired? + long now = System.currentTimeMillis(); + if (s.isExpiredAt(now)) + return; + + if (s.isValid() && s.isIdleLongerThan(_sessionStore.getIdlePassivationTimeoutSec())) + { + _idleCandidates.add(s.getId()); + }; + } + + + /** + * @return the idleCandidates + */ + public Set getIdleCandidates() + { + return _idleCandidates; + } + + + /** + * @see org.eclipse.jetty.server.session.SessionInspector#preInspection() + */ + @Override + public void preInspection() + { + _idleCandidates = new HashSet(); + } + + + + /** + * @see org.eclipse.jetty.server.session.SessionInspector#postInspection() + */ + @Override + public void postInspection() + { + for (String id:_idleCandidates) + { + try + { + _sessionStore.passivateIdleSession(id); + } + catch (Exception e) + { + LOG.warn(e); + } + } + + _idleCandidates = null; + } +} 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 6a82f9da872..ebce29c1c29 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 @@ -50,7 +50,7 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr protected final HashSet _sessionIds = new HashSet(); protected Server _server; - protected SessionScavenger _scavenger; + protected PeriodicSessionInspector _scavenger; private DatabaseAdaptor _dbAdaptor = new DatabaseAdaptor(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/MemorySessionStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/MemorySessionStore.java index a4d62fbdd4a..6ecf9021fdb 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/MemorySessionStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/MemorySessionStore.java @@ -22,6 +22,7 @@ package org.eclipse.jetty.server.session; import java.util.HashSet; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Stream; import javax.servlet.http.HttpServletRequest; @@ -150,7 +151,7 @@ public class MemorySessionStore extends AbstractSessionStore } - +/* @Override public Set doGetExpiredCandidates() @@ -167,7 +168,7 @@ public class MemorySessionStore extends AbstractSessionStore } return candidates; } - +*/ @@ -242,4 +243,14 @@ public class MemorySessionStore extends AbstractSessionStore } + /** + * @see org.eclipse.jetty.server.session.SessionStore#getStream() + */ + @Override + public Stream getStream() + { + return _sessions.values().stream(); + } + + } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionScavenger.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/PeriodicSessionInspector.java similarity index 79% rename from jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionScavenger.java rename to jetty-server/src/main/java/org/eclipse/jetty/server/session/PeriodicSessionInspector.java index d5acdb759c8..c10520dfb70 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionScavenger.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/PeriodicSessionInspector.java @@ -35,25 +35,26 @@ import org.eclipse.jetty.util.thread.Scheduler; * There is 1 session scavenger per SessionIdManager/Server instance. * */ -public class SessionScavenger extends AbstractLifeCycle +public class PeriodicSessionInspector extends AbstractLifeCycle { private final static Logger LOG = Log.getLogger("org.eclipse.jetty.server.session"); - public static final long DEFAULT_SCAVENGE_MS = 1000L * 60 * 10; + public static final long DEFAULT_PERIOD_MS = 1000L * 60 * 10; protected SessionIdManager _sessionIdManager; protected Scheduler _scheduler; protected Scheduler.Task _task; //scavenge task - protected ScavengerRunner _runner; + protected Runner _runner; protected boolean _ownScheduler = false; - private long _scavengeIntervalMs = DEFAULT_SCAVENGE_MS; - + private long _intervalMs = DEFAULT_PERIOD_MS; + private long _lastTime = 0L; + /** - * ScavengerRunner + * Runner * */ - protected class ScavengerRunner implements Runnable + protected class Runner implements Runnable { @Override @@ -61,12 +62,12 @@ public class SessionScavenger extends AbstractLifeCycle { try { - scavenge(); + inspect(); } finally { if (_scheduler != null && _scheduler.isRunning()) - _task = _scheduler.schedule(this, _scavengeIntervalMs, TimeUnit.MILLISECONDS); + _task = _scheduler.schedule(this, _intervalMs, TimeUnit.MILLISECONDS); } } } @@ -96,6 +97,7 @@ public class SessionScavenger extends AbstractLifeCycle if (!(_sessionIdManager instanceof AbstractSessionIdManager)) throw new IllegalStateException ("SessionIdManager is not an AbstractSessionIdManager"); + _lastTime = System.currentTimeMillis(); //set it to a non zero value //try and use a common scheduler, fallback to own _scheduler = ((AbstractSessionIdManager)_sessionIdManager).getServer().getBean(Scheduler.class); @@ -109,7 +111,7 @@ public class SessionScavenger extends AbstractLifeCycle else if (!_scheduler.isStarted()) throw new IllegalStateException("Shared scheduler not started"); - setScavengeIntervalSec(getScavengeIntervalSec()); + setIntervalSec(getIntervalSec()); super.doStart(); } @@ -138,24 +140,24 @@ public class SessionScavenger extends AbstractLifeCycle * Set the period between scavenge cycles * @param sec */ - public void setScavengeIntervalSec (long sec) + public void setIntervalSec (long sec) { if (sec<=0) sec=60; - long old_period=_scavengeIntervalMs; + long old_period=_intervalMs; long period=sec*1000L; - _scavengeIntervalMs=period; + _intervalMs=period; //add a bit of variability into the scavenge time so that not all //nodes with the same scavenge interval sync up - long tenPercent = _scavengeIntervalMs/10; + long tenPercent = _intervalMs/10; if ((System.currentTimeMillis()%2) == 0) - _scavengeIntervalMs += tenPercent; + _intervalMs += tenPercent; if (LOG.isDebugEnabled()) - LOG.debug("Scavenging every "+_scavengeIntervalMs+" ms"); + LOG.debug("Inspecting every "+_intervalMs+" ms"); synchronized (this) { @@ -164,8 +166,8 @@ public class SessionScavenger extends AbstractLifeCycle if (_task!=null) _task.cancel(); if (_runner == null) - _runner = new ScavengerRunner(); - _task = _scheduler.schedule(_runner,_scavengeIntervalMs,TimeUnit.MILLISECONDS); + _runner = new Runner(); + _task = _scheduler.schedule(_runner,_intervalMs,TimeUnit.MILLISECONDS); } } } @@ -173,13 +175,13 @@ public class SessionScavenger extends AbstractLifeCycle /** - * Get the period between scavenge cycles. + * Get the period between inspection cycles. * * @return */ - public long getScavengeIntervalSec () + public long getIntervalSec () { - return _scavengeIntervalMs/1000; + return _intervalMs/1000; } @@ -189,26 +191,33 @@ public class SessionScavenger extends AbstractLifeCycle * ask all SessionManagers to find sessions they think have expired and then make * sure that a session sharing the same id is expired on all contexts */ - public void scavenge () + public void inspect () { //don't attempt to scavenge if we are shutting down if (isStopping() || isStopped()) return; if (LOG.isDebugEnabled()) - LOG.debug("Scavenging sessions"); + LOG.debug("Inspecting sessions"); + long now = System.currentTimeMillis(); + //find the session managers for (SessionManager manager:((AbstractSessionIdManager)_sessionIdManager).getSessionManagers()) { if (manager != null) { + manager.inspect(); + + /* //call scavenge on each manager to find keys for sessions that have expired Set expiredKeys = manager.scavenge(); //for each expired session, tell the session id manager to invalidate its key on all contexts for (String key:expiredKeys) { + + // if it recently expired try { ((AbstractSessionIdManager)_sessionIdManager).expireAll(key); @@ -217,7 +226,7 @@ public class SessionScavenger extends AbstractLifeCycle { LOG.warn(e); } - } + }*/ } } } @@ -229,7 +238,7 @@ public class SessionScavenger extends AbstractLifeCycle @Override public String toString() { - return super.toString()+"[interval="+_scavengeIntervalMs+", ownscheduler="+_ownScheduler+"]"; + return super.toString()+"[interval="+_intervalMs+", ownscheduler="+_ownScheduler+"]"; } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java index 43dd9a80604..d23d627c76a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java @@ -65,6 +65,9 @@ public class Session implements SessionManager.SessionIf private boolean _newSession; private State _state = State.VALID; //state of the session:valid,invalid or being invalidated private Locker _lock = new Locker(); + + + private boolean _isPassivated; public Session (HttpServletRequest request, SessionData data) { @@ -147,6 +150,17 @@ public class Session implements SessionManager.SessionIf } } + + protected boolean isIdleLongerThan (int sec) + { + long now = System.currentTimeMillis(); + try (Lock lock = _lock.lockIfNotHeld()) + { + return ((_sessionData.getAccessed() + (sec*1000)) < now); + } + } + + /* ------------------------------------------------------------ */ /** * @param nodename @@ -520,7 +534,7 @@ public class Session implements SessionManager.SessionIf public void setAttribute(String name, Object value) { Object old=null; - try (Lock lock = lock()) + try (Lock lock = _lock.lockIfNotHeld()) { //if session is not valid, don't accept the set checkValidForWrite(); @@ -578,7 +592,7 @@ public class Session implements SessionManager.SessionIf String id = null; String extendedId = null; - try (Lock lock = lock()) + try (Lock lock = _lock.lockIfNotHeld()) { checkValidForWrite(); //don't renew id on a session that is not valid id = _sessionData.getId(); //grab the values as they are now @@ -601,7 +615,7 @@ public class Session implements SessionManager.SessionIf */ public void renewId (String oldId, String oldExtendedId, String newId, String newExtendedId) { - try (Lock lock = lock()) + try (Lock lock = _lock.lockIfNotHeld()) { checkValidForWrite(); //can't change id on invalid session @@ -632,7 +646,7 @@ public class Session implements SessionManager.SessionIf boolean result = false; - try (Lock lock = lock()) + try (Lock lock = _lock.lockIfNotHeld()) { switch (_state) { @@ -669,7 +683,7 @@ public class Session implements SessionManager.SessionIf } } - + /* ------------------------------------------------------------- */ /** Grab the lock on the session @@ -687,7 +701,7 @@ public class Session implements SessionManager.SessionIf /* ------------------------------------------------------------- */ protected void doInvalidate() throws IllegalStateException { - try (Lock lock = lock()) + try (Lock lock = _lock.lockIfNotHeld()) { try { @@ -769,4 +783,26 @@ public class Session implements SessionManager.SessionIf { return _sessionData; } + + + /* ------------------------------------------------------------- */ + /** + * @return + */ + public boolean isPassivated() + { + return _isPassivated; + } + + /* ------------------------------------------------------------- */ + /** + * @param isPassivated + */ + public void setPassivated(boolean isPassivated) + { + _isPassivated = isPassivated; + } + + + } 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 7d0ea1b0ebf..3f05b08aca1 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 @@ -83,6 +83,36 @@ public class SessionData implements Serializable _maxInactiveMs = maxInactiveMs; } + + /** + * Copy the info from the given sessiondata + * + * @param data the sessiondata to be copied + */ + public void copy (SessionData data) + { + if (data == null) + return; //don't copy if no data + + if (data.getId() == null || !(getId().equals(data.getId()))) + throw new IllegalStateException ("Can only copy data for same session id"); + + if (data == this) + return; //don't copy ourself + + setLastNode(data.getLastNode()); + setContextPath(data.getContextPath()); + setVhost(data.getVhost()); + setCookieSet(data.getCookieSet()); + setCreated(data.getCreated()); + setAccessed(data.getAccessed()); + setLastAccessed(data.getLastAccessed()); + setMaxInactiveMs(data.getMaxInactiveMs()); + setExpiry(data.getExpiry()); + setLastSaved(data.getLastSaved()); + clearAllAttributes(); + putAllAttributes(data.getAllAttributes()); + } /** * @return time at which session was last written out @@ -170,6 +200,14 @@ public class SessionData implements Serializable _attributes.putAll(attributes); } + /** + * Remove all attributes + */ + public void clearAllAttributes () + { + _attributes.clear(); + } + /** * @return an unmodifiable map of the attributes */ @@ -322,12 +360,6 @@ public class SessionData implements Serializable _lastAccessed = lastAccessed; } - /* public boolean isInvalid() - { - return _invalid; - } -*/ - /** * @return diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionInspector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionInspector.java new file mode 100644 index 00000000000..e70c372b5cf --- /dev/null +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionInspector.java @@ -0,0 +1,34 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + + +package org.eclipse.jetty.server.session; + + + +/** + * SessionInspector + * + * + */ +public interface SessionInspector +{ + public void preInspection(); + public void inspect(Session s); + public void postInspection (); //on completion +} 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 ea9683d920a..746a90f0f20 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 @@ -1017,13 +1017,14 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je /** * @return */ - public Set scavenge () + public void inspect () { //don't attempt to scavenge if we are shutting down if (isStopping() || isStopped()) - return Collections.emptySet(); + return; - return _sessionStore.getExpired(); + _sessionStore.inspect(); + //return _sessionStore.getExpired(); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionStore.java index 214c7787c6a..2ab2fc06e88 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionStore.java @@ -19,7 +19,9 @@ package org.eclipse.jetty.server.session; +import java.util.List; import java.util.Set; +import java.util.stream.Stream; import javax.servlet.http.HttpServletRequest; @@ -43,5 +45,9 @@ public interface SessionStore extends LifeCycle boolean exists (String id) throws Exception; Session delete (String id) throws Exception; void shutdown (); - Set getExpired (); + Set checkExpiration (Set candidates); + void inspect(); + void setIdlePassivationTimeoutSec(int sec); + int getIdlePassivationTimeoutSec(); + Stream getStream(); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java index 0a1b3ce84c9..b6d83e9ae1c 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/session/SessionCookieTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue; import java.util.Enumeration; import java.util.Map; import java.util.Set; +import java.util.stream.Stream; import javax.servlet.SessionCookieConfig; import javax.servlet.http.HttpServletRequest; @@ -113,14 +114,16 @@ public class SessionCookieTest } /** - * @see org.eclipse.jetty.server.session.AbstractSessionStore#doGetExpiredCandidates() + * @see org.eclipse.jetty.server.session.SessionStore#getStream() */ @Override - public Set doGetExpiredCandidates() + public Stream getStream() { // TODO Auto-generated method stub return null; - } + } + + } diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractImmortalSessionTest.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractImmortalSessionTest.java index 4bfb0472f06..d031e4ec706 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractImmortalSessionTest.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractImmortalSessionTest.java @@ -20,6 +20,7 @@ package org.eclipse.jetty.server.session; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotNull; import java.io.IOException; import java.io.PrintWriter; @@ -76,7 +77,7 @@ public abstract class AbstractImmortalSessionTest // Let's wait for the scavenger to run, waiting 2.5 times the scavenger period Thread.sleep(scavengePeriod * 2500L); - + // Be sure the session is still there Request request = client.newRequest("http://localhost:" + port + contextPath + servletMapping + "?action=get"); request.header("Cookie", sessionCookie); @@ -114,7 +115,8 @@ public abstract class AbstractImmortalSessionTest } else if ("get".equals(action)) { - HttpSession session = request.getSession(false); + HttpSession session = request.getSession(false); + assertNotNull(session); if (session!=null) result = (String)session.getAttribute("value"); } diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractTestServer.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractTestServer.java index 04e8bb99577..2b922b7b539 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractTestServer.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractTestServer.java @@ -42,7 +42,7 @@ public abstract class AbstractTestServer protected final int _scavengePeriod; protected final ContextHandlerCollection _contexts; protected SessionIdManager _sessionIdManager; - private SessionScavenger _scavenger; + private PeriodicSessionInspector _scavenger; @@ -83,8 +83,8 @@ public abstract class AbstractTestServer _sessionIdManager = newSessionIdManager(sessionIdMgrConfig); _server.setSessionIdManager(_sessionIdManager); ((AbstractSessionIdManager) _sessionIdManager).setServer(_server); - _scavenger = new SessionScavenger(); - _scavenger.setScavengeIntervalSec(scavengePeriod); + _scavenger = new PeriodicSessionInspector(); + _scavenger.setIntervalSec(scavengePeriod); ((AbstractSessionIdManager)_sessionIdManager).setSessionScavenger(_scavenger); }