From 4ce2104a70b8add0767fc2fad4fbd1c3dd40e6e2 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 18 Dec 2015 12:10:50 +1100 Subject: [PATCH] Resolve threading issues for simultaneous session invalidate. --- .../session/GCloudSessionIdManager.java | 9 +- .../InfinispanSessionIdManager.java | 8 +- .../nosql/mongodb/MongoSessionIdManager.java | 3 +- .../jetty/server/SessionIdManager.java | 3 +- .../session/AbstractSessionIdManager.java | 67 ++-- .../server/session/AbstractSessionStore.java | 38 +- .../server/session/HashSessionIdManager.java | 14 +- .../server/session/JDBCSessionDataStore.java | 2 +- .../server/session/JDBCSessionIdManager.java | 11 +- .../server/session/MemorySessionStore.java | 4 +- .../eclipse/jetty/server/session/Session.java | 356 ++++++++++++------ .../jetty/server/session/SessionData.java | 12 +- .../jetty/server/session/SessionManager.java | 52 +-- .../jetty/server/session/SessionStore.java | 2 +- .../server/session/SessionCookieTest.java | 9 +- 15 files changed, 367 insertions(+), 223 deletions(-) diff --git a/jetty-gcloud/gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionIdManager.java b/jetty-gcloud/gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionIdManager.java index f290f8137dc..3a38f52f2b1 100644 --- a/jetty-gcloud/gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionIdManager.java +++ b/jetty-gcloud/gcloud-session-manager/src/main/java/org/eclipse/jetty/gcloud/session/GCloudSessionIdManager.java @@ -158,12 +158,13 @@ public class GCloudSessionIdManager extends AbstractSessionIdManager * * @param id */ - protected void delete (String id) + protected boolean delete (String id) { if (_datastore == null) throw new IllegalStateException ("No DataStore"); _datastore.delete(makeKey(id)); + return true; //gcloud does not distinguish between first and subsequent removes } @@ -220,11 +221,11 @@ public class GCloudSessionIdManager extends AbstractSessionIdManager * @see org.eclipse.jetty.server.SessionIdManager#removeId(java.lang.String) */ @Override - public void removeId(String id) + public boolean removeId(String id) { if (id == null) - return; + return false; - delete(id); + return delete(id); } } 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 b2a1d35ec25..365a5926750 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 @@ -243,12 +243,12 @@ public class InfinispanSessionIdManager extends AbstractSessionIdManager * * @param id the session id */ - protected void delete (String id) + protected boolean delete (String id) { if (_cache == null) throw new IllegalStateException ("No cache"); - _cache.remove(makeKey(id)); + return _cache.remove(makeKey(id)) != null; } @@ -283,8 +283,8 @@ public class InfinispanSessionIdManager extends AbstractSessionIdManager * @see org.eclipse.jetty.server.SessionIdManager#removeId(java.lang.String) */ @Override - public void removeId(String id) + public boolean removeId(String id) { - delete (id); + return delete (id); } } diff --git a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionIdManager.java b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionIdManager.java index 24a3e04f249..9113d9c8963 100644 --- a/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionIdManager.java +++ b/jetty-nosql/src/main/java/org/eclipse/jetty/nosql/mongodb/MongoSessionIdManager.java @@ -167,9 +167,10 @@ public class MongoSessionIdManager extends AbstractSessionIdManager /* ------------------------------------------------------------ */ @Override - public void removeId(String id) + public boolean removeId(String id) { //The corresponding session document will be marked as expired or invalid? + return true; //can't distinguish first remove vs subsequent removes } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/SessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/SessionIdManager.java index 13619af1269..755b863a226 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/SessionIdManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/SessionIdManager.java @@ -44,8 +44,9 @@ public interface SessionIdManager extends LifeCycle /** * Remove id * @param id the plain session id (no workername extension) of the session to remove + * @return true if the id was removed, false otherwise */ - public void removeId (String id); + public boolean removeId (String id); /** * Invalidate all sessions on all contexts that share the same 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 0a0f9e921e0..aa5e723e3af 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 @@ -19,7 +19,9 @@ package org.eclipse.jetty.server.session; import java.security.SecureRandom; +import java.util.HashSet; import java.util.Random; +import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -322,47 +324,29 @@ public abstract class AbstractSessionIdManager extends AbstractLifeCycle impleme public void expireAll(String id) { //take the id out of the list of known sessionids for this node - removeId(id); - - //tell all contexts that may have a session object with this id to - //get rid of them - Handler[] contexts = _server.getChildHandlersByClass(ContextHandler.class); - for (int i=0; contexts!=null && i getSessionManagers() + { + Set managers = new HashSet<>(); + Handler[] contexts = _server.getChildHandlersByClass(ContextHandler.class); for (int i=0; contexts!=null && i)o); 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 55d2a3df9d8..534f2243f1f 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 @@ -197,11 +197,11 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr * @see org.eclipse.jetty.server.SessionIdManager#removeId(java.lang.String) */ @Override - public void removeId (String id) + public boolean removeId (String id) { if (id == null) - return; + return false; synchronized (_sessionIds) { @@ -210,11 +210,12 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr try { _sessionIds.remove(id); - delete(id); + return delete(id); } catch (Exception e) { LOG.warn("Problem removing session id="+id, e); + return false; } } @@ -255,7 +256,7 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr * @param id * @throws SQLException */ - private void delete (String id) + private boolean delete (String id) throws SQLException { try (Connection connection = _dbAdaptor.getConnection(); @@ -263,7 +264,7 @@ public class JDBCSessionIdManager extends org.eclipse.jetty.server.session.Abstr { connection.setAutoCommit(true); statement.setString(1, id); - statement.executeUpdate(); + return (statement.executeUpdate() > 0); } } 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 e0e5a94f4b6..9ed35053322 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 @@ -146,12 +146,12 @@ public class MemorySessionStore extends AbstractSessionStore * @see org.eclipse.jetty.server.session.AbstractSessionStore#doDelete(java.lang.String) */ @Override - public boolean doDelete(String id) + public Session doDelete(String id) { Session s = _sessions.remove(id); if (s != null) _stats.decrement(); - return (s != null); + return s; } 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 77e63611b6a..06694bc5e24 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 @@ -20,9 +20,11 @@ package org.eclipse.jetty.server.session; import java.util.ArrayList; +import java.util.Collections; import java.util.Enumeration; import java.util.Iterator; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; @@ -34,6 +36,8 @@ import javax.servlet.http.HttpSessionEvent; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.thread.Locker; +import org.eclipse.jetty.util.thread.Locker.Lock; @@ -50,13 +54,18 @@ public class Session implements SessionManager.SessionIf public final static String SESSION_CREATED_SECURE="org.eclipse.jetty.security.sessionCreatedSecure"; + + public enum State {VALID, INVALID, INVALIDATING}; + + protected SessionData _sessionData; protected SessionManager _manager; protected String _extendedId; //the _id plus the worker name protected long _requests; private boolean _idChanged; private boolean _newSession; - private boolean _doInvalidate; //remember we should invalidate this session + private State _state = State.VALID; //state of the session:valid,invalid or being invalidated + private Locker _lock = new Locker(); public Session (HttpServletRequest request, SessionData data) { @@ -87,7 +96,7 @@ public class Session implements SessionManager.SessionIf /* ------------------------------------------------------------- */ protected void cookieSet() { - synchronized (this) + try (Lock lock = lock()) { _sessionData.setCookieSet(_sessionData.getAccessed()); } @@ -95,7 +104,7 @@ public class Session implements SessionManager.SessionIf /* ------------------------------------------------------------ */ protected boolean access(long time) { - synchronized(this) + try (Lock lock=lock()) { if (!isValid()) return false; @@ -118,30 +127,14 @@ public class Session implements SessionManager.SessionIf /* ------------------------------------------------------------ */ protected void complete() { - synchronized(this) + try (Lock lock = lock()) { _requests--; - if (_doInvalidate && _requests<=0 ) - doInvalidate(); } } + - /* ------------------------------------------------------------- */ - /** - * - * @throws Exception - */ - protected void invalidateAndRemove() throws Exception - { - if (_manager == null) - throw new IllegalStateException ("No session manager for session "+ _sessionData.getId()); - - _manager.removeSession(this,true); - doInvalidate(); - } - - /* ------------------------------------------------------------- */ /** Check to see if session has expired as at the time given. * @param time the time in milliseconds @@ -149,22 +142,35 @@ public class Session implements SessionManager.SessionIf */ protected boolean isExpiredAt(long time) { - return _sessionData.isExpiredAt(time); + try (Lock lock = lock()) + { + return _sessionData.isExpiredAt(time); + } } - + /* ------------------------------------------------------------ */ + /** + * @param nodename + */ public void setLastNode (String nodename) { _sessionData.setLastNode(nodename); } - + /* ------------------------------------------------------------ */ + /** + * @return + */ public String getLastNode () { return _sessionData.getLastNode(); } - public void clearAttributes() + /* ------------------------------------------------------------ */ + /** + * + */ + protected void clearAttributes() { Set keys = null; @@ -173,13 +179,30 @@ public class Session implements SessionManager.SessionIf keys = _sessionData.getKeys(); for (String key:keys) { - setAttribute(key,null); + clearAttribute(key); } } while (!keys.isEmpty()); } + /* ------------------------------------------------------------ */ + /** Unset the attribute. + * @param name + */ + protected void clearAttribute (String name) + { + Object old=null; + try (Lock lock = lock()) + { + //if session is not valid, don't accept the set + + old=_sessionData.setAttribute(name,null); + } + if (old == null) + return; //if same as remove attribute but attribute was already removed, no change + callSessionAttributeListeners(name, null, old); + } /* ------------------------------------------------------------ */ /** @@ -231,73 +254,79 @@ public class Session implements SessionManager.SessionIf if (value!=null&&value instanceof HttpSessionBindingListener) ((HttpSessionBindingListener)value).valueBound(new HttpSessionBindingEvent(this,name)); } - + /* ------------------------------------------------------------- */ + /** + * Call the activation listeners. This must be called holding the + * _lock. + */ public void didActivate() { - synchronized(this) + HttpSessionEvent event = new HttpSessionEvent(this); + for (Iterator iter = _sessionData.getKeys().iterator(); iter.hasNext();) { - HttpSessionEvent event = new HttpSessionEvent(this); - for (Iterator iter = _sessionData.getKeys().iterator(); iter.hasNext();) + Object value = _sessionData.getAttribute(iter.next()); + if (value instanceof HttpSessionActivationListener) { - Object value = _sessionData.getAttribute(iter.next()); - if (value instanceof HttpSessionActivationListener) - { - HttpSessionActivationListener listener = (HttpSessionActivationListener) value; - listener.sessionDidActivate(event); - } + HttpSessionActivationListener listener = (HttpSessionActivationListener) value; + listener.sessionDidActivate(event); } } } - - + + /* ------------------------------------------------------------- */ + /** + * Call the passivation listeners. This must be called holding the + * _lock + */ public void willPassivate() { - synchronized(this) + HttpSessionEvent event = new HttpSessionEvent(this); + for (Iterator iter = _sessionData.getKeys().iterator(); iter.hasNext();) { - HttpSessionEvent event = new HttpSessionEvent(this); - for (Iterator iter = _sessionData.getKeys().iterator(); iter.hasNext();) + Object value = _sessionData.getAttribute(iter.next()); + if (value instanceof HttpSessionActivationListener) { - Object value = _sessionData.getAttribute(iter.next()); - if (value instanceof HttpSessionActivationListener) - { - HttpSessionActivationListener listener = (HttpSessionActivationListener) value; - listener.sessionWillPassivate(event); - } + HttpSessionActivationListener listener = (HttpSessionActivationListener) value; + listener.sessionWillPassivate(event); } } } - + /* ------------------------------------------------------------ */ public boolean isValid() { - return !_sessionData.isInvalid(); + try (Lock lock = lock()) + { + return _state==State.VALID; + } } /* ------------------------------------------------------------- */ public long getCookieSetTime() { - return _sessionData.getCookieSet(); - } - - /* ------------------------------------------------------------- */ - public void setCookieSetTime(long time) - { - _sessionData.setCookieSet(time); + try (Lock lock = lock()) + { + return _sessionData.getCookieSet(); + } } + /* ------------------------------------------------------------- */ @Override public long getCreationTime() throws IllegalStateException { - checkValid(); - return _sessionData.getCreated(); + try (Lock lock = lock()) + { + checkValid(); + return _sessionData.getCreated(); + } } - - + + /** * @see javax.servlet.http.HttpSession#getId() @@ -305,9 +334,12 @@ public class Session implements SessionManager.SessionIf @Override public String getId() { - return _sessionData.getId(); + try (Lock lock = lock()) + { + return _sessionData.getId(); + } } - + public String getExtendedId() { @@ -325,14 +357,17 @@ public class Session implements SessionManager.SessionIf return _sessionData.getVhost(); } - + /** * @see javax.servlet.http.HttpSession#getLastAccessedTime() */ @Override public long getLastAccessedTime() { - return _sessionData.getLastAccessed(); + try (Lock lock = lock()) + { + return _sessionData.getLastAccessed(); + } } /** @@ -352,9 +387,12 @@ public class Session implements SessionManager.SessionIf @Override public void setMaxInactiveInterval(int secs) { - _sessionData.setMaxInactiveMs((long)secs*1000L); - _sessionData.setExpiry(_sessionData.getMaxInactiveMs() <= 0 ? 0 : (System.currentTimeMillis() + _sessionData.getMaxInactiveMs()*1000L)); - _sessionData.setDirty(true); + try (Lock lock = lock()) + { + _sessionData.setMaxInactiveMs((long)secs*1000L); + _sessionData.setExpiry(_sessionData.getMaxInactiveMs() <= 0 ? 0 : (System.currentTimeMillis() + _sessionData.getMaxInactiveMs()*1000L)); + _sessionData.setDirty(true); + } } /** @@ -363,7 +401,10 @@ public class Session implements SessionManager.SessionIf @Override public int getMaxInactiveInterval() { - return (int)(_sessionData.getMaxInactiveMs()/1000); + try (Lock lock = lock()) + { + return (int)(_sessionData.getMaxInactiveMs()/1000); + } } /** @@ -386,24 +427,29 @@ public class Session implements SessionManager.SessionIf /* ------------------------------------------------------------- */ /** * asserts that the session is valid - * @throws IllegalStateException if the sesion is invalid + * @throws IllegalStateException if the session is invalid */ protected void checkValid() throws IllegalStateException - { - if (_sessionData.isInvalid()) + { + if (!_lock.isLocked()) + throw new IllegalStateException(); + + if (_state != State.VALID) throw new IllegalStateException(); } - - + + /** * @see javax.servlet.http.HttpSession#getAttribute(java.lang.String) */ @Override public Object getAttribute(String name) { - //TODO synchronization - checkValid(); - return _sessionData.getAttribute(name); + try (Lock lock = lock()) + { + checkValid(); + return _sessionData.getAttribute(name); + } } /** @@ -412,8 +458,10 @@ public class Session implements SessionManager.SessionIf @Override public Object getValue(String name) { - // TODO synchronization - return _sessionData.getAttribute(name); + try (Lock lock = lock()) + { + return _sessionData.getAttribute(name); + } } /** @@ -422,7 +470,7 @@ public class Session implements SessionManager.SessionIf @Override public Enumeration getAttributeNames() { - synchronized (this) + try (Lock lock = lock()) { checkValid(); final Iterator itor = _sessionData.getKeys().iterator(); @@ -446,24 +494,24 @@ public class Session implements SessionManager.SessionIf } - + /* ------------------------------------------------------------ */ public int getAttributes() { return _sessionData.getKeys().size(); } - - + + /* ------------------------------------------------------------ */ public Set getNames() { - return _sessionData.getKeys(); + return Collections.unmodifiableSet(_sessionData.getKeys()); } - - + + /* ------------------------------------------------------------- */ /** * @deprecated As of Version 2.2, this method is replaced by @@ -473,7 +521,7 @@ public class Session implements SessionManager.SessionIf @Override public String[] getValueNames() throws IllegalStateException { - synchronized(this) + try (Lock lock = lock()) { checkValid(); Iterator itor = _sessionData.getKeys().iterator(); @@ -486,7 +534,7 @@ public class Session implements SessionManager.SessionIf } } - + /* ------------------------------------------------------------- */ /** * @see javax.servlet.http.HttpSession#setAttribute(java.lang.String, java.lang.Object) */ @@ -494,7 +542,7 @@ public class Session implements SessionManager.SessionIf public void setAttribute(String name, Object value) { Object old=null; - synchronized (this) + try (Lock lock = lock()) { //if session is not valid, don't accept the set checkValid(); @@ -504,7 +552,10 @@ public class Session implements SessionManager.SessionIf return; //if same as remove attribute but attribute was already removed, no change callSessionAttributeListeners(name, value, old); } - + + + + /* ------------------------------------------------------------- */ /** * @see javax.servlet.http.HttpSession#putValue(java.lang.String, java.lang.Object) */ @@ -513,7 +564,10 @@ public class Session implements SessionManager.SessionIf { setAttribute(name,value); } - + + + + /* ------------------------------------------------------------- */ /** * @see javax.servlet.http.HttpSession#removeAttribute(java.lang.String) */ @@ -522,7 +576,10 @@ public class Session implements SessionManager.SessionIf { setAttribute(name, null); } - + + + + /* ------------------------------------------------------------- */ /** * @see javax.servlet.http.HttpSession#removeValue(java.lang.String) */ @@ -538,7 +595,21 @@ public class Session implements SessionManager.SessionIf if (_manager == null) throw new IllegalStateException ("No session manager for session "+ _sessionData.getId()); - _manager._sessionIdManager.renewSessionId(_sessionData.getId(), getExtendedId(), request); + //TODO + //simultaneous calls to renew id on same session - all subsequent calls to renew need to wait + //until first renew finishes - and then should they proceed in order, or does first renew win???? + //Or are simultaneous interleaved renewals ok, so long as we end up with a consistent result??? + + String id = null; + String extendedId = null; + try (Lock lock = lock()) + { + checkValid(); //don't renew id on a session that is not valid + id = _sessionData.getId(); //grab the values as they are now + extendedId = getExtendedId(); + } + + _manager._sessionIdManager.renewSessionId(id, extendedId, request); setIdChanged(true); } @@ -546,7 +617,8 @@ public class Session implements SessionManager.SessionIf /* ------------------------------------------------------------- */ /** Called by users to invalidate a session, or called by the * access method as a request enters the session if the session - * has expired. + * has expired, or called by manager as a result of scavenger + * expiring session * * @see javax.servlet.http.HttpSession#invalidate() */ @@ -555,59 +627,115 @@ public class Session implements SessionManager.SessionIf { if (_manager == null) throw new IllegalStateException ("No session manager for session "+ _sessionData.getId()); - - checkValid(); + + boolean result = false; + + try (Lock lock = lock()) + { + switch (_state) + { + case INVALID: + { + throw new IllegalStateException(); //spec does not allow invalidate of already invalid session + } + case VALID: + { + //only first change from valid to invalidating should be actionable + result = true; + _state = State.INVALIDATING; + break; + } + default: + { + LOG.info("Session {} already being invalidated", _sessionData.getId()); + } + } + } + try { - //tell id mgr to remove session from all other contexts - ((AbstractSessionIdManager)_manager.getSessionIdManager()).invalidateAll(_sessionData.getId()); - + //if the session was not already invalid, or in process of being invalidated, do invalidate + if (result) + { + //tell id mgr to remove session from all other contexts + ((AbstractSessionIdManager)_manager.getSessionIdManager()).invalidateAll(_sessionData.getId()); + } } catch (Exception e) { LOG.warn(e); } } + + + + /* ------------------------------------------------------------- */ + /** Grab the lock on the session + * @return + */ + public Lock lock () + { + return _lock.lock(); + } + + + + /* ------------------------------------------------------------- */ protected void doInvalidate() throws IllegalStateException { - try + try (Lock lock = lock()) { - if (LOG.isDebugEnabled()) - LOG.debug("invalidate {}",_sessionData.getId()); - if (isValid()) - clearAttributes(); - } - finally - { - synchronized (this) + try + { + if (LOG.isDebugEnabled()) + LOG.debug("invalidate {}",_sessionData.getId()); + if (isValid()) + clearAttributes(); + } + finally { // mark as invalid - _sessionData.setInvalid(true); + _state = State.INVALID; } } } - + /* ------------------------------------------------------------- */ @Override public boolean isNew() throws IllegalStateException { - checkValid(); - return _newSession; + try (Lock lock = lock()) + { + checkValid(); + return _newSession; + } } + + /* ------------------------------------------------------------- */ public void setIdChanged(boolean changed) { - _idChanged=changed; + try (Lock lock = lock()) + { + _idChanged=changed; + } } + + /* ------------------------------------------------------------- */ public boolean isIdChanged () { - return _idChanged; + try (Lock lock = lock()) + { + return _idChanged; + } } + + /* ------------------------------------------------------------- */ /** * @see org.eclipse.jetty.server.session.AbstractSessionManager.SessionIf#getSession() */ @@ -618,7 +746,7 @@ public class Session implements SessionManager.SessionIf return this; } - + /* ------------------------------------------------------------- */ protected SessionData getSessionData() { return _sessionData; 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 761caa496b2..1cebc42be7b 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 @@ -38,6 +38,7 @@ import java.util.concurrent.ConcurrentHashMap; public class SessionData implements Serializable { + private static final long serialVersionUID = 1L; protected String _id; @@ -53,7 +54,7 @@ public class SessionData implements Serializable protected long _cookieSet; protected long _accessed; // the time of the last access protected long _lastAccessed; // the time of the last access excluding this one - protected boolean _invalid; + // protected boolean _invalid; protected long _maxInactiveMs; protected Map _attributes = new ConcurrentHashMap(); protected boolean _dirty; @@ -232,15 +233,12 @@ public class SessionData implements Serializable _lastAccessed = lastAccessed; } - public boolean isInvalid() + /* public boolean isInvalid() { return _invalid; } - - public void setInvalid(boolean invalid) - { - _invalid = invalid; - } +*/ + public long getMaxInactiveMs() { 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 9fe1125346f..3721c767252 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 @@ -55,7 +55,7 @@ import org.eclipse.jetty.util.statistic.CounterStatistic; import org.eclipse.jetty.util.statistic.SampleStatistic; /** - * AbstractSessionManager + * SessionManager * * */ @@ -748,7 +748,10 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je } - + /* ------------------------------------------------------------ */ + /** + * @return + */ public SessionStore getSessionStore () { return _sessionStore; @@ -776,18 +779,18 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je /* ------------------------------------------------------------ */ /** * Remove session from manager - * @param session The session to remove + * @param id The session to remove * @param invalidate True if {@link HttpSessionListener#sessionDestroyed(HttpSessionEvent)} and * {@link SessionIdManager#expireAll(String)} should be called. * @return if the session was removed */ - public boolean removeSession(Session session, boolean invalidate) + public Session removeSession(String id, boolean invalidate) { try { //Remove the Session object from the session store and any backing data store - boolean removed = _sessionStore.delete(session.getId()); - if (removed) + Session session = _sessionStore.delete(id); + if (session != null) { if (invalidate) { @@ -802,12 +805,12 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je } } - return removed; + return session; } catch (Exception e) { LOG.warn(e); - return false; + return null; } } @@ -939,11 +942,9 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je session.getSessionData().setId(newId); session.setExtendedId(newExtendedId); session.getSessionData().setLastSaved(0); //forces an insert - session.getSessionData().setDirty(true); + session.getSessionData().setDirty(true); //forces an insert _sessionStore.put(newId, session); - - Session x = _sessionStore.get(newId, false); - + //tell session id manager the id is in use _sessionIdManager.useId(session); @@ -968,7 +969,13 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je - + /* ------------------------------------------------------------ */ + /** + * Called either when a session has expired, or the app has + * invalidated it. + * + * @param id + */ public void invalidate (String id) { if (StringUtil.isBlank(id)) @@ -976,23 +983,26 @@ public class SessionManager extends ContainerLifeCycle implements org.eclipse.je try { - Session session = _sessionStore.get(id, false); - if (session == null) + //remove the session and call the destroy listeners + Session session = removeSession(id, true); + + if (session != null) { - return; // couldn't get/load a session for this context with that id + _sessionTimeStats.set(round((System.currentTimeMillis() - session.getSessionData().getCreated())/1000.0)); + session.doInvalidate(); } - - _sessionTimeStats.set(round((System.currentTimeMillis() - session.getSessionData().getCreated())/1000.0)); - session.invalidateAndRemove(); } catch (Exception e) { LOG.warn(e); } } + - - + /* ------------------------------------------------------------ */ + /** + * @return + */ public Set scavenge () { //don't attempt to scavenge if we are shutting down 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 31c72400bec..a7251929806 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 @@ -41,7 +41,7 @@ public interface SessionStore extends LifeCycle Session get(String id, boolean staleCheck) throws Exception; void put(String id, Session session) throws Exception; boolean exists (String id) throws Exception; - boolean delete (String id) throws Exception; + Session delete (String id) throws Exception; void shutdown (); Set getExpired (); } 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 b7e75827688..2869657b595 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 @@ -107,9 +107,9 @@ public class SessionCookieTest * @see org.eclipse.jetty.server.session.AbstractSessionStore#doDelete(org.eclipse.jetty.server.session.SessionKey) */ @Override - public boolean doDelete(String key) + public Session doDelete(String key) { - return false; + return null; } /** @@ -175,10 +175,9 @@ public class SessionCookieTest * @see org.eclipse.jetty.server.SessionIdManager#removeId(java.lang.String) */ @Override - public void removeId(String id) + public boolean removeId(String id) { - // TODO Auto-generated method stub - + return true; } }