Issue #3913 Fix races in session request reference counting (#3947)

* Issue #3913 Fix races in session request reference counting

Signed-off-by: Jan Bartel <janb@webtide.com>

jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java

@@ -759,8 +759,8 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor
         if (idleTO >= 0 && getIdleTimeout() != _oldIdleTimeout)
             setIdleTimeout(_oldIdleTimeout);
 
+        _request.onCompleted();
         notifyComplete(_request);
-
         _transport.onCompleted();
     }
 

jetty-server/src/main/java/org/eclipse/jetty/server/Request.java

@@ -222,6 +222,7 @@ public class Request implements HttpServletRequest
     private long _timeStamp;
     private MultiParts _multiParts; //if the request is a multi-part mime
     private AsyncContextState _async;
+    private List<HttpSession> _sessions; //list of sessions used during lifetime of request
 
     public Request(HttpChannel channel, HttpInput input)
     {
@@ -368,6 +369,39 @@ public class Request implements HttpServletRequest
         if (listener instanceof AsyncListener)
             throw new IllegalArgumentException(listener.getClass().toString());
     }
+    
+    /**
+     * Remember a session that this request has just entered.
+     * 
+     * @param s the session
+     */
+    public void enterSession(HttpSession s)
+    {
+        if (s == null)
+            return;
+
+        if (_sessions == null)
+            _sessions = new ArrayList<>();
+        if (LOG.isDebugEnabled())
+            LOG.debug("Request {} entering session={}", this, s);
+        _sessions.add(s);
+    }
+
+    /**
+     * Complete this request's access to a session.
+     *
+     * @param s the session
+     */
+    private void leaveSession(HttpSession s)
+    {
+        if (s == null)
+            return;
+        
+        Session session = (Session)s;
+        if (LOG.isDebugEnabled())
+            LOG.debug("Request {} leaving session {}", this, session);
+        session.getSessionHandler().complete(session);
+    }
 
     private MultiMap<String> getParameters()
     {
@@ -1467,6 +1501,46 @@ public class Request implements HttpServletRequest
         return session.getId();
     }
 
+    /**
+     * Called when the request is fully finished being handled.
+     * For every session in any context that the session has
+     * accessed, ensure that the session is completed.
+     */
+    public void onCompleted()
+    {
+        if (_sessions != null && _sessions.size() > 0)
+        {
+            for (HttpSession s:_sessions)
+                leaveSession(s);
+        }
+    }
+    
+    /**
+     * Find a session that this request has already entered for the
+     * given SessionHandler 
+     *
+     * @param sessionHandler the SessionHandler (ie context) to check
+     * @return
+     */
+    public HttpSession getSession(SessionHandler sessionHandler)
+    {
+        if (_sessions == null || _sessions.size() == 0 || sessionHandler == null)
+            return null;
+        
+        HttpSession session = null;
+        
+        for (HttpSession s:_sessions)
+        {
+            Session ss =  Session.class.cast(s);
+            if (sessionHandler == ss.getSessionHandler())
+            {
+                session = s;
+                break;
+            }
+        }
+        return session;
+    }
+    
     /*
      * @see javax.servlet.http.HttpServletRequest#getSession()
      */
@@ -1789,6 +1863,7 @@ public class Request implements HttpServletRequest
         _inputState = INPUT_NONE;
         _multiParts = null;
         _remote = null;
+        _sessions = null;
         _input.recycle();
     }
 

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

@@ -111,12 +111,13 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
     public abstract Session newSession(HttpServletRequest request, SessionData data);
 
     /**
-     * Get the session matching the key
+     * Get the session matching the key from the cache. Does not load
+     * the session.
      *
      * @param id session id
      * @return the Session object matching the id
      */
-    public abstract Session doGet(String id);
+    protected abstract Session doGet(String id);
 
     /**
      * Put the session into the map if it wasn't already there
@@ -125,7 +126,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
      * @param session the session object
      * @return null if the session wasn't already in the map, or the existing entry otherwise
      */
-    public abstract Session doPutIfAbsent(String id, Session session);
+    protected abstract Session doPutIfAbsent(String id, Session session);
 
     /**
      * Replace the mapping from id to oldValue with newValue
@@ -135,7 +136,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
      * @param newValue the new value
      * @return true if replacement was done
      */
-    public abstract boolean doReplace(String id, Session oldValue, Session newValue);
+    protected abstract boolean doReplace(String id, Session oldValue, Session newValue);
 
     /**
      * Remove the session with this identity from the store
@@ -300,12 +301,28 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
      *
      * If the session object is not in this session store, try getting
      * the data for it from a SessionDataStore associated with the
-     * session manager.
+     * session manager. The usage count of the session is incremented.
      *
      * @see org.eclipse.jetty.server.session.SessionCache#get(java.lang.String)
      */
     @Override
     public Session get(String id) throws Exception
+    {
+        return getAndEnter(id, true);
+    }
+
+    /** Get a session object.
+     *
+     * If the session object is not in this session store, try getting
+     * the data for it from a SessionDataStore associated with the
+     * session manager.
+     * 
+     * @param id The session to retrieve
+     * @param enter if true, the usage count of the session will be incremented
+     * @return
+     * @throws Exception
+     */
+    protected Session getAndEnter(String id, boolean enter) throws Exception
     {
         Session session = null;
         Exception ex = null;
@@ -320,7 +337,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
             if (session == null)
             {
                 if (LOG.isDebugEnabled())
-                    LOG.debug("Session {} not found locally, attempting to load", id);
+                    LOG.debug("Session {} not found locally in {}, attempting to load", id, this);
 
                 //didn't get a session, try and create one and put in a placeholder for it
                 PlaceHolderSession phs = new PlaceHolderSession(_handler, new SessionData(id, null, null, 0, 0, 0, 0));
@@ -357,6 +374,8 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
                             {
                                 //successfully swapped in the session
                                 session.setResident(true);
+                                if (enter)
+                                    session.use();
                                 phsLock.close();
                                 break;
                             }
@@ -383,7 +402,10 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
                             session = null;
                             continue;
                         }
+                        //I will use this session too
                         session = s;
+                        if (enter)
+                            session.use();
                         break;
                     }
                 }
@@ -401,6 +423,8 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
                     }
 
                     //got the session
+                    if (enter)
+                        session.use();
                     break;
                 }
             }
@@ -447,7 +471,47 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
     }
 
     /**
-     * Put the Session object back into the session store.
+     * Add an entirely new session (created by the application calling Request.getSession(true))
+     * to the cache. The usage count of the fresh session is incremented.
+     * 
+     * @param id the id
+     * @param session
+     */
+    @Override
+    public void add(String id, Session session) throws Exception
+    {
+        if (id == null || session == null)
+            throw new IllegalArgumentException("Add key=" + id + " session=" + (session == null ? "null" : session.getId()));
+
+        try (Lock lock = session.lock())
+        {
+            if (session.getSessionHandler() == null)
+                throw new IllegalStateException("Session " + id + " is not managed");
+
+            if (!session.isValid())
+                throw new IllegalStateException("Session " + id + " is not valid");
+
+            if (doPutIfAbsent(id, session) == null)
+            {
+                session.setResident(true); //its in the cache
+                session.use(); //the request is using it
+            }
+            else
+                throw new IllegalStateException("Session " + id + " already in cache");
+        }
+    }
+
+    /**
+     * @deprecated
+     */
+    @Override
+    public void put(String id, Session session) throws Exception
+    {
+        release(id, session);
+    }
+
+    /**
+     * Finish using the Session object.
      *
      * This should be called when a request exists the session. Only when the last
      * simultaneous request exists the session will any action be taken.
@@ -459,10 +523,10 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
      * If the evictionPolicy == SessionCache.EVICT_ON_SESSION_EXIT then after we have saved
      * the session, we evict it from the cache.
      *
-     * @see org.eclipse.jetty.server.session.SessionCache#put(java.lang.String, org.eclipse.jetty.server.session.Session)
+     * @see org.eclipse.jetty.server.session.SessionCache#release(java.lang.String, org.eclipse.jetty.server.session.Session)
      */
     @Override
-    public void put(String id, Session session) throws Exception
+    public void release(String id, Session session) throws Exception
     {
         if (id == null || session == null)
             throw new IllegalArgumentException("Put key=" + id + " session=" + (session == null ? "null" : session.getId()));
@@ -472,9 +536,11 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
             if (session.getSessionHandler() == null)
                 throw new IllegalStateException("Session " + id + " is not managed");
 
-            if (!session.isValid())
+            if (session.isInvalid())
                 return;
 
+            session.complete();
+
             //don't do anything with the session until the last request for it has finished
             if ((session.getRequests() <= 0))
             {
@@ -586,7 +652,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
     public Session delete(String id) throws Exception
     {
         //get the session, if its not in memory, this will load it
-        Session session = get(id);
+        Session session = getAndEnter(id, false);
 
         //Always delete it from the backing data store
         if (_sessionDataStore != null)
@@ -657,7 +723,8 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
             LOG.debug("Checking for idle {}", session.getId());
         try (Lock s = session.lock())
         {
-            if (getEvictionPolicy() > 0 && session.isIdleLongerThan(getEvictionPolicy()) && session.isValid() && session.isResident() && session.getRequests() <= 0)
+            if (getEvictionPolicy() > 0 && session.isIdleLongerThan(getEvictionPolicy())
+                && session.isValid() && session.isResident() && session.getRequests() <= 0)
             {
                 //Be careful with saveOnInactiveEviction - you may be able to re-animate a session that was
                 //being managed on another node and has expired.
@@ -696,7 +763,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
         if (StringUtil.isBlank(newId))
             throw new IllegalArgumentException("New session id is null");
 
-        Session session = get(oldId);
+        Session session = getAndEnter(oldId, true);
         renewSessionId(session, newId, newExtendedId);
 
         return session;
@@ -787,6 +854,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
     public String toString()
     {
         return String.format("%s@%x[evict=%d,removeUnloadable=%b,saveOnCreate=%b,saveOnInactiveEvict=%b]",
-            this.getClass().getName(), this.hashCode(), _evictionPolicy, _removeUnloadableSessions, _saveOnCreate, _saveOnInactiveEviction);
+            this.getClass().getName(), this.hashCode(), _evictionPolicy,
+            _removeUnloadableSessions, _saveOnCreate, _saveOnInactiveEviction);
     }
 }

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

@@ -84,6 +84,7 @@ public class Session implements SessionHandler.SessionIf
     protected final SessionHandler _handler; // the manager of the session
 
     protected String _extendedId; // the _id plus the worker name
+
     protected long _requests;
 
     protected boolean _idChanged;
@@ -127,7 +128,8 @@ public class Session implements SessionHandler.SessionIf
                     {
                         //grab the lock and check what happened to the session: if it didn't get evicted and
                         //it hasn't expired, we need to reset the timer
-                        if (Session.this.isResident() && Session.this.getRequests() <= 0 && Session.this.isValid() && !Session.this.isExpiredAt(now))
+                        if (Session.this.isResident() && Session.this.getRequests() <= 0 && Session.this.isValid() &&
+                            !Session.this.isExpiredAt(now))
                         {
                             //session wasn't expired or evicted, we need to reset the timer
                             SessionInactivityTimer.this.schedule(Session.this.calculateInactivityTimeout(now));
@@ -195,8 +197,6 @@ public class Session implements SessionHandler.SessionIf
         _sessionData = data;
         _newSession = true;
         _sessionData.setDirty(true);
-        _requests = 1; // access will not be called on this new session, but we
-        // are obviously in a request
         _sessionInactivityTimer = new SessionInactivityTimer();
     }
 
@@ -239,11 +239,24 @@ public class Session implements SessionHandler.SessionIf
         }
     }
 
+    protected void use()
+    {
+        try (Lock lock = _lock.lock())
+        {
+            _requests++;
+
+            // temporarily stop the idle timer
+            if (LOG.isDebugEnabled())
+                LOG.debug("Session {} in use, stopping timer, active requests={}", getId(), _requests);
+            _sessionInactivityTimer.cancel();
+        }
+    }
+
     protected boolean access(long time)
     {
         try (Lock lock = _lock.lock())
         {
-            if (!isValid())
+            if (!isValid() || !isResident())
                 return false;
             _newSession = false;
             long lastAccessed = _sessionData.getAccessed();
@@ -255,13 +268,6 @@ public class Session implements SessionHandler.SessionIf
                 invalidate();
                 return false;
             }
-            _requests++;
-
-            // temporarily stop the idle timer
-            if (LOG.isDebugEnabled())
-                LOG.debug("Session {} accessed, stopping timer, active requests={}", getId(), _requests);
-            _sessionInactivityTimer.cancel();
-
             return true;
         }
     }
@@ -374,7 +380,7 @@ public class Session implements SessionHandler.SessionIf
     public void didActivate()
     {
         HttpSessionEvent event = new HttpSessionEvent(this);
-        for (Iterator<String> iter = _sessionData.getKeys().iterator(); iter.hasNext(); )
+        for (Iterator<String> iter = _sessionData.getKeys().iterator(); iter.hasNext();)
         {
             Object value = _sessionData.getAttribute(iter.next());
             if (value instanceof HttpSessionActivationListener)
@@ -391,7 +397,7 @@ public class Session implements SessionHandler.SessionIf
     public void willPassivate()
     {
         HttpSessionEvent event = new HttpSessionEvent(this);
-        for (Iterator<String> iter = _sessionData.getKeys().iterator(); iter.hasNext(); )
+        for (Iterator<String> iter = _sessionData.getKeys().iterator(); iter.hasNext();)
         {
             Object value = _sessionData.getAttribute(iter.next());
             if (value instanceof HttpSessionActivationListener)
@@ -410,6 +416,14 @@ public class Session implements SessionHandler.SessionIf
         }
     }
 
+    public boolean isInvalid()
+    {
+        try (Lock lock = _lock.lock())
+        {
+            return _state == State.INVALID || _state == State.INVALIDATING;
+        }
+    }
+
     public boolean isChanging()
     {
         checkLocked();
@@ -574,7 +588,8 @@ public class Session implements SessionHandler.SessionIf
                     time = (remaining > 0 ? (Math.min(maxInactive, TimeUnit.SECONDS.toMillis(evictionPolicy))) : 0);
 
                     if (LOG.isDebugEnabled())
-                        LOG.debug("Session {} timer set to lesser of maxInactive={} and inactivityEvict={}", getId(), maxInactive, evictionPolicy);
+                        LOG.debug("Session {} timer set to lesser of maxInactive={} and inactivityEvict={}", getId(),
+                            maxInactive, evictionPolicy);
                 }
             }
         }
@@ -965,6 +980,8 @@ public class Session implements SessionHandler.SessionIf
                     {
                         try
                         {
+                            if (LOG.isDebugEnabled())
+                                LOG.debug("Session {} waiting for id change to complete", _sessionData.getId());
                             _stateChangeCompleted.await();
                         }
                         catch (InterruptedException e)

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

@@ -118,6 +118,16 @@ public interface SessionCache extends LifeCycle
     {
         return renewSessionId(oldId, newId);
     }
+    
+    /**
+     * Adds a new Session, with a never-before-used id,
+     *  to the cache.
+     * 
+     * @param id
+     * @param session
+     * @throws Exception
+     */
+    void add(String id, Session session) throws Exception;
 
     /**
      * Get an existing Session. If necessary, the cache will load the data for
@@ -138,9 +148,24 @@ public interface SessionCache extends LifeCycle
      * @param id the session id
      * @param session the current session object
      * @throws Exception if any error occurred
+     * @deprecated @see release
      */
     void put(String id, Session session) throws Exception;
-
+    
+    
+    /**
+     * Finish using a Session. This is called by the SessionHandler
+     * once a request is finished with a Session. SessionCache
+     * implementations may want to delay writing out Session contents
+     * until the last request exits a Session.
+     *
+     * @param id the session id
+     * @param session the current session object
+     * @throws Exception if any error occurred
+     */
+    void release(String id, Session session) throws Exception;
+    
+    
     /**
      * Check to see if a Session is in the cache. Does NOT consult
      * the SessionDataStore.

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

@@ -18,6 +18,8 @@
 
 package org.eclipse.jetty.server.session;
 
+import static java.lang.Math.round;
+
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
@@ -29,8 +31,7 @@ import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
-import javax.servlet.AsyncEvent;
-import javax.servlet.AsyncListener;
+
 import javax.servlet.DispatcherType;
 import javax.servlet.ServletException;
 import javax.servlet.SessionCookieConfig;
@@ -64,8 +65,6 @@ import org.eclipse.jetty.util.thread.Locker.Lock;
 import org.eclipse.jetty.util.thread.ScheduledExecutorScheduler;
 import org.eclipse.jetty.util.thread.Scheduler;
 
-import static java.lang.Math.round;
-
 /**
  * SessionHandler.
  */
@@ -74,7 +73,8 @@ public class SessionHandler extends ScopedHandler
 {
     static final Logger LOG = Log.getLogger("org.eclipse.jetty.server.session");
 
-    public static final EnumSet<SessionTrackingMode> DEFAULT_TRACKING = EnumSet.of(SessionTrackingMode.COOKIE, SessionTrackingMode.URL);
+    public static final EnumSet<SessionTrackingMode> DEFAULT_TRACKING = EnumSet.of(SessionTrackingMode.COOKIE,
+        SessionTrackingMode.URL);
 
     /**
      * Session cookie name.
@@ -127,7 +127,8 @@ public class SessionHandler extends ScopedHandler
 
     @SuppressWarnings("unchecked")
     public static final Class<? extends EventListener>[] SESSION_LISTENER_TYPES =
-        new Class[]{
+        new Class[]
+        {
             HttpSessionAttributeListener.class,
             HttpSessionIdListener.class,
             HttpSessionListener.class
@@ -140,54 +141,6 @@ public class SessionHandler extends ScopedHandler
      */
     public static final java.math.BigDecimal MAX_INACTIVE_MINUTES = new java.math.BigDecimal(Integer.MAX_VALUE / 60);
 
-    /**
-     * SessionAsyncListener
-     *
-     * Used to ensure that a request for which async has been started
-     * has its session completed as the request exits the context.
-     */
-    public class SessionAsyncListener implements AsyncListener
-    {
-        @Override
-        public void onComplete(AsyncEvent event) throws IOException
-        {
-            // An async request has completed, so we can complete the session,
-            // but we must locate the session instance for this context
-            Request request = Request.getBaseRequest(event.getAsyncContext().getRequest());
-            HttpSession session = request.getSession(false);
-            String id;
-            if (session != null)
-                id = session.getId();
-            else
-            {
-                id = (String)request.getAttribute(DefaultSessionIdManager.__NEW_SESSION_ID);
-                if (id == null)
-                    id = request.getRequestedSessionId();
-            }
-
-            if (id != null)
-                complete(getSession(id));
-        }
-
-        @Override
-        public void onTimeout(AsyncEvent event) throws IOException
-        {
-
-        }
-
-        @Override
-        public void onError(AsyncEvent event) throws IOException
-        {
-            complete(Request.getBaseRequest(event.getAsyncContext().getRequest()).getSession(false));
-        }
-
-        @Override
-        public void onStartAsync(AsyncEvent event) throws IOException
-        {
-            event.getAsyncContext().addListener(this);
-        }
-    }
-
     static final HttpSessionContext __nullSessionContext = new HttpSessionContext()
     {
         @Override
@@ -243,7 +196,6 @@ public class SessionHandler extends ScopedHandler
 
     protected Scheduler _scheduler;
     protected boolean _ownScheduler = false;
-    protected final SessionAsyncListener _sessionAsyncListener = new SessionAsyncListener();
 
     /**
      * Constructor.
@@ -285,9 +237,8 @@ public class SessionHandler extends ScopedHandler
             // Do we need to refresh the cookie?
             if (isUsingCookies() &&
                 (s.isIdChanged() ||
-                    (getSessionCookieConfig().getMaxAge() > 0 && getRefreshCookieAge() > 0 && ((now - s.getCookieSetTime()) / 1000 > getRefreshCookieAge()))
-                )
-            )
+                    (getSessionCookieConfig().getMaxAge() > 0 && getRefreshCookieAge() > 0 
+                        && ((now - s.getCookieSetTime())/1000 > getRefreshCookieAge()))))
             {
                 HttpCookie cookie = getSessionCookie(session, _context == null ? "/" : (_context.getContextPath()), secure);
                 s.cookieSet();
@@ -413,11 +364,9 @@ public class SessionHandler extends ScopedHandler
             return;
 
         Session s = ((SessionIf)session).getSession();
-
         try
         {
-            s.complete();
-            _sessionCache.put(s.getId(), s);
+            _sessionCache.release(s.getId(), s);
         }
         catch (Exception e)
         {
@@ -428,22 +377,7 @@ public class SessionHandler extends ScopedHandler
     @Deprecated
     public void complete(Session session, Request baseRequest)
     {
-        ensureCompletion(baseRequest);
-    }
-
-    private void ensureCompletion(Request baseRequest)
-    {
-        if (baseRequest.isAsyncStarted())
-        {
-            if (LOG.isDebugEnabled())
-                LOG.debug("Adding AsyncListener for {}", baseRequest);
-            if (!baseRequest.getHttpChannelState().hasListener(_sessionAsyncListener))
-                baseRequest.getAsyncContext().addListener(_sessionAsyncListener);
-        }
-        else
-        {
-            complete(baseRequest.getSession(false));
-        }
+        //not used
     }
 
     /*
@@ -581,11 +515,11 @@ public class SessionHandler extends ScopedHandler
      * @param extendedId the session id
      * @return the <code>HttpSession</code> with the corresponding id or null if no session with the given id exists
      */
-    public HttpSession getHttpSession(String extendedId)
+    protected HttpSession getHttpSession(String extendedId)
     {
         String id = getSessionIdManager().getId(extendedId);
-
         Session session = getSession(id);
+
         if (session != null && !session.getExtendedId().equals(extendedId))
             session.setIdChanged(true);
         return session;
@@ -813,7 +747,8 @@ public class SessionHandler extends ScopedHandler
 
         try
         {
-            _sessionCache.put(id, session);
+            _sessionCache.add(id, session);
+            Request.getBaseRequest(request).enterSession(session);
             _sessionsCreatedStats.increment();
 
             if (request != null && request.isSecure())
@@ -912,7 +847,8 @@ public class SessionHandler extends ScopedHandler
     public void setSessionIdPathParameterName(String param)
     {
         _sessionIdPathParameterName = (param == null || "none".equals(param)) ? null : param;
-        _sessionIdPathParameterNamePrefix = (param == null || "none".equals(param)) ? null : (";" + _sessionIdPathParameterName + "=");
+        _sessionIdPathParameterNamePrefix = (param == null || "none".equals(param)) ? 
+                                            null : (";" + _sessionIdPathParameterName + "=");
     }
 
     /**
@@ -929,7 +865,7 @@ public class SessionHandler extends ScopedHandler
      * @param id The session ID stripped of any worker name.
      * @return A Session or null if none exists.
      */
-    public Session getSession(String id)
+    protected Session getSession(String id)
     {
         try
         {
@@ -953,7 +889,6 @@ public class SessionHandler extends ScopedHandler
                 }
 
                 session.setExtendedId(_sessionIdManager.getExtendedId(id, null));
-                //session.getSessionData().setLastNode(_sessionIdManager.getWorkerName());  //TODO write through the change of node?
             }
             return session;
         }
@@ -1159,9 +1094,11 @@ public class SessionHandler extends ScopedHandler
      */
     public void renewSessionId(String oldId, String oldExtendedId, String newId, String newExtendedId)
     {
+        Session session = null;
         try
         {
-            Session session = _sessionCache.renewSessionId(oldId, newId, oldExtendedId, newExtendedId); //swap the id over
+            //the use count for the session will be incremented in renewSessionId
+            session = _sessionCache.renewSessionId(oldId, newId, oldExtendedId, newExtendedId); //swap the id over
             if (session == null)
             {
                 //session doesn't exist on this context
@@ -1175,6 +1112,20 @@ public class SessionHandler extends ScopedHandler
         {
             LOG.warn(e);
         }
+        finally
+        {
+            if (session != null)
+            {
+                try
+                {
+                    _sessionCache.release(newId, session);
+                }
+                catch (Exception e)
+                {
+                    LOG.warn(e);
+                }
+            }
+        }
     }
 
     /**
@@ -1337,7 +1288,8 @@ public class SessionHandler extends ScopedHandler
                 //most efficient if it can be done as a bulk operation to eg reduce
                 //roundtrips to the persistent store. Only do this if the HouseKeeper that
                 //does the scavenging is configured to actually scavenge
-                if (_sessionIdManager.getSessionHouseKeeper() != null && _sessionIdManager.getSessionHouseKeeper().getIntervalSec() > 0)
+                if (_sessionIdManager.getSessionHouseKeeper() != null
+                    && _sessionIdManager.getSessionHouseKeeper().getIntervalSec() > 0)
                 {
                     _candidateSessionIdsForExpiry.add(session.getId());
                     if (LOG.isDebugEnabled())
@@ -1509,7 +1461,8 @@ public class SessionHandler extends ScopedHandler
      * @see org.eclipse.jetty.server.Handler#handle(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse, int)
      */
     @Override
-    public void doScope(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
+    public void doScope(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response)
+        throws IOException, ServletException
     {
         SessionHandler oldSessionHandler = null;
         HttpSession oldSession = null;
@@ -1518,27 +1471,57 @@ public class SessionHandler extends ScopedHandler
         try
         {
             if (LOG.isDebugEnabled())
-                LOG.debug("SessionHandler.doScope");
+                LOG.debug("Entering scope {}, dispatch={} asyncstarted={}", this, baseRequest.getDispatcherType(), baseRequest
+                    .isAsyncStarted());
 
-            oldSessionHandler = baseRequest.getSessionHandler();
-            oldSession = baseRequest.getSession(false);
-
-            if (oldSessionHandler != this)
+            switch (baseRequest.getDispatcherType())
             {
-                // new session context
-                baseRequest.setSessionHandler(this);
-                baseRequest.setSession(null);
-                checkRequestedSessionId(baseRequest, request);
-            }
+                case REQUEST:
+                {
+                    //there are no previous sessionhandlers or sessions for dispatch=REQUEST
+                    //look for a session for this context
+                    baseRequest.setSession(null);
+                    checkRequestedSessionId(baseRequest, request);
+                    existingSession = baseRequest.getSession(false);
+                    baseRequest.setSessionHandler(this);
+                    baseRequest.setSession(existingSession); //can be null
+                    break;
+                }
+                case ASYNC:
+                case ERROR:
+                case FORWARD:
+                case INCLUDE:
+                {
+                    //remember previous sessionhandler and session
+                    oldSessionHandler = baseRequest.getSessionHandler();
+                    oldSession = baseRequest.getSession(false);
 
-            // access any existing session for this context
-            existingSession = baseRequest.getSession(false);
+                    //find any existing session for this request that has already been accessed
+                    existingSession = baseRequest.getSession(this);
+                    if (existingSession == null)
+                    {
+                        //session for this context has not been visited previously,
+                        //try getting it
+                        baseRequest.setSession(null);
+                        checkRequestedSessionId(baseRequest, request);
+                        existingSession = baseRequest.getSession(false);
+                    }
+
+                    baseRequest.setSession(existingSession);
+                    baseRequest.setSessionHandler(this);
+                    break;
+                }
+                default:
+                    break;
+            }
 
             if ((existingSession != null) && (oldSessionHandler != this))
             {
                 HttpCookie cookie = access(existingSession, request.isSecure());
                 // Handle changed ID or max-age refresh, but only if this is not a redispatched request
-                if ((cookie != null) && (request.getDispatcherType() == DispatcherType.ASYNC || request.getDispatcherType() == DispatcherType.REQUEST))
+                if ((cookie != null) 
+                    && (request.getDispatcherType() == DispatcherType.ASYNC
+                        || request.getDispatcherType() == DispatcherType.REQUEST))
                     baseRequest.getResponse().replaceCookie(cookie);
             }
 
@@ -1554,13 +1537,10 @@ public class SessionHandler extends ScopedHandler
         }
         finally
         {
-            //if there is a session that was created during handling this context, then complete it
             if (LOG.isDebugEnabled())
-                LOG.debug("FinalSession={}, old_session_handler={}, this={}, calling complete={}", baseRequest.getSession(false), oldSessionHandler, this, (oldSessionHandler != this));
-
-            // If we are leaving the scope of this session handler, ensure the session is completed
-            if (oldSessionHandler != this)
-                ensureCompletion(baseRequest);
+                LOG.debug("Leaving scope {} dispatch={}, async={}, session={}, oldsession={}, oldsessionhandler={}",
+                    this, baseRequest.getDispatcherType(), baseRequest.isAsyncStarted(), baseRequest.getSession(false),
+                    oldSession, oldSessionHandler);
 
             // revert the session handler to the previous, unless it was null, in which case remember it as
             // the first session handler encountered.
@@ -1576,7 +1556,8 @@ public class SessionHandler extends ScopedHandler
      * @see org.eclipse.jetty.server.Handler#handle(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse, int)
      */
     @Override
-    public void doHandle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
+    public void doHandle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response)
+        throws IOException, ServletException
     {
         nextHandle(target, baseRequest, request, response);
     }
@@ -1596,7 +1577,10 @@ public class SessionHandler extends ScopedHandler
             HttpSession session = getHttpSession(requestedSessionId);
 
             if (session != null && isValid(session))
+            {
+                baseRequest.enterSession(session); //enter session for first time
                 baseRequest.setSession(session);
+            }
             return;
         }
         else if (!DispatcherType.REQUEST.equals(baseRequest.getDispatcherType()))
@@ -1605,7 +1589,7 @@ public class SessionHandler extends ScopedHandler
         boolean requestedSessionIdFromCookie = false;
         HttpSession session = null;
 
-        // Look for session id cookie
+        //first try getting id from a cookie
         if (isUsingCookies())
         {
             Cookie[] cookies = request.getCookies();
@@ -1618,31 +1602,21 @@ public class SessionHandler extends ScopedHandler
                     {
                         requestedSessionId = cookies[i].getValue();
                         requestedSessionIdFromCookie = true;
-
                         if (LOG.isDebugEnabled())
-                            LOG.debug("Got Session ID {} from cookie", requestedSessionId);
-
+                            LOG.debug("Got Session ID {} from cookie {}", requestedSessionId, sessionCookie);
                         if (requestedSessionId != null)
                         {
-                            session = getHttpSession(requestedSessionId);
-                            if (session != null && isValid(session))
-                            {
-                                break;
-                            }
-                        }
-                        else
-                        {
-                            LOG.warn("null session id from cookie");
+                            break;
                         }
                     }
                 }
             }
         }
 
-        if (isUsingURLs() && (requestedSessionId == null || session == null))
+        //try getting id from a url
+        if (isUsingURLs() && (requestedSessionId == null))
         {
             String uri = request.getRequestURI();
-
             String prefix = getSessionIdPathParameterNamePrefix();
             if (prefix != null)
             {
@@ -1661,7 +1635,6 @@ public class SessionHandler extends ScopedHandler
 
                     requestedSessionId = uri.substring(s, i);
                     requestedSessionIdFromCookie = false;
-                    session = getHttpSession(requestedSessionId);
                     if (LOG.isDebugEnabled())
                         LOG.debug("Got Session ID {} from URL", requestedSessionId);
                 }
@@ -1670,8 +1643,16 @@ public class SessionHandler extends ScopedHandler
 
         baseRequest.setRequestedSessionId(requestedSessionId);
         baseRequest.setRequestedSessionIdFromCookie(requestedSessionId != null && requestedSessionIdFromCookie);
-        if (session != null && isValid(session))
-            baseRequest.setSession(session);
+
+        if (requestedSessionId != null)
+        {
+            session = getHttpSession(requestedSessionId);
+            if (session != null && isValid(session))
+            {
+                baseRequest.enterSession(session); //request enters this session for first time
+                baseRequest.setSession(session);
+            }
+        }
     }
 
     /**

tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ClusteredSessionMigrationTest.java

@@ -68,6 +68,7 @@ public class ClusteredSessionMigrationTest extends AbstractTestBase
 
         DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
         cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        cacheFactory.setSaveOnCreate(true); //immediately save the session when it is created so node2 can see it
         SessionDataStoreFactory storeFactory = createSessionDataStoreFactory();
         ((AbstractSessionDataStoreFactory)storeFactory).setGracePeriodSec(TestServer.DEFAULT_SCAVENGE_SEC);
 

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractClusteredSessionScavengingTest.java

@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
@@ -76,6 +77,8 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
         TestServlet servlet1 = new TestServlet();
         ServletHolder holder1 = new ServletHolder(servlet1);
         ServletContextHandler context = server1.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        context.addEventListener(scopeListener);
         TestSessionListener listener1 = new TestSessionListener();
         context.getSessionHandler().addEventListener(listener1);
         context.addServlet(holder1, servletMapping);
@@ -93,6 +96,8 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
             ((AbstractSessionDataStoreFactory)storeFactory2).setSavePeriodSec(0); //always save when the session exits
             TestServer server2 = new TestServer(0, maxInactivePeriod, scavengePeriod, cacheFactory2, storeFactory2);
             ServletContextHandler context2 = server2.addContext(contextPath);
+            TestContextScopeListener scopeListener2 = new TestContextScopeListener();
+            context2.addEventListener(scopeListener2);
             context2.addServlet(TestServlet.class, servletMapping);
             SessionHandler m2 = context2.getSessionHandler();
 
@@ -105,18 +110,26 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
                 try
                 {
                     // Perform one request to server1 to create a session
+                    CountDownLatch synchronizer = new CountDownLatch(1);
+                    scopeListener.setExitSynchronizer(synchronizer);
                     ContentResponse response1 = client.GET("http://localhost:" + port1 + contextPath + servletMapping.substring(1) + "?action=init");
                     assertEquals(HttpServletResponse.SC_OK, response1.getStatus());
-                    assertEquals("test", response1.getContentAsString());
+                    assertTrue(response1.getContentAsString().startsWith("init"));
                     String sessionCookie = response1.getHeaders().get("Set-Cookie");
                     assertTrue(sessionCookie != null);
+
+                    //ensure request has finished being handled
+                    synchronizer.await(5, TimeUnit.SECONDS);
+                    
                     assertEquals(1, ((DefaultSessionCache)m1.getSessionCache()).getSessionsCurrent());
                     assertEquals(1, ((DefaultSessionCache)m1.getSessionCache()).getSessionsMax());
                     assertEquals(1, ((DefaultSessionCache)m1.getSessionCache()).getSessionsTotal());
                     // Mangle the cookie, replacing Path with $Path, etc.
                     sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
                     String id = TestServer.extractSessionId(sessionCookie);
-                    Session s1 = m1.getSessionCache().get(id);
+                    
+                    //Peek at the contents of the cache without doing all the reference counting etc
+                    Session s1 = ((AbstractSessionCache)m1.getSessionCache()).doGet(id);
                     assertNotNull(s1);
                     long expiry = s1.getSessionData().getExpiry();
 
@@ -128,11 +141,15 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
                     long time = start;
                     while (time < end)
                     {
+                        synchronizer = new CountDownLatch(1);
+                        scopeListener2.setExitSynchronizer(synchronizer);
                         Request request = client.newRequest("http://localhost:" + port2 + contextPath + servletMapping.substring(1));
                         request.header("Cookie", sessionCookie); //use existing session
                         ContentResponse response2 = request.send();
                         assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
-                        assertEquals("test", response2.getContentAsString());
+                        assertTrue(response2.getContentAsString().startsWith("test"));
+                        //ensure request has finished being handled
+                        synchronizer.await(5, TimeUnit.SECONDS);
                         Thread.sleep(requestInterval);
                         assertSessionCounts(1, 1, 1, m2);
                         time = System.currentTimeMillis();
@@ -208,20 +225,22 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
             if ("init".equals(action))
             {
                 HttpSession session = request.getSession(true);
-                session.setAttribute("test", "test");
+                session.setAttribute("test", "init");
                 sendResult(session, httpServletResponse.getWriter());
             }
             else
             {
                 HttpSession session = request.getSession(false);
 
-                // if we node hopped we should get the session and test should already be present
-                sendResult(session, httpServletResponse.getWriter());
 
                 if (session != null)
                 {
                     session.setAttribute("test", "test");
                 }
+                
+                // if we node hopped we should get the session and test should already be present
+                sendResult(session, httpServletResponse.getWriter());
+
             }
         }
 
@@ -229,11 +248,11 @@ public abstract class AbstractClusteredSessionScavengingTest extends AbstractTes
         {
             if (session != null)
             {
-                writer.print(session.getAttribute("test"));
+                writer.println(session.getAttribute("test"));
             }
             else
             {
-                writer.print("null");
+                writer.println("null");
             }
         }
     }

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractWebAppObjectInSessionTest.java

@@ -21,6 +21,9 @@ package org.eclipse.jetty.server.session;
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.FileWriter;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
 import javax.servlet.http.HttpServletResponse;
 
 import org.eclipse.jetty.client.HttpClient;
@@ -29,6 +32,7 @@ import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.resource.Resource;
+import org.eclipse.jetty.webapp.WebAppContext;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -98,7 +102,10 @@ public abstract class AbstractWebAppObjectInSessionTest extends AbstractTestBase
 
         TestServer server1 = new TestServer(0, TestServer.DEFAULT_MAX_INACTIVE, TestServer.DEFAULT_SCAVENGE_SEC,
             cacheFactory, storeFactory);
-        server1.addWebAppContext(warDir.getCanonicalPath(), contextPath).addServlet(WebAppObjectInSessionServlet.class.getName(), servletMapping);
+        WebAppContext wac1 = server1.addWebAppContext(warDir.getCanonicalPath(), contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        wac1.addEventListener(scopeListener);
+        wac1.addServlet(WebAppObjectInSessionServlet.class.getName(), servletMapping);
 
         try
         {
@@ -119,16 +126,20 @@ public abstract class AbstractWebAppObjectInSessionTest extends AbstractTestBase
                 try
                 {
                     // Perform one request to server1 to create a session
+                    CountDownLatch synchronizer = new CountDownLatch(1);
+                    scopeListener.setExitSynchronizer(synchronizer);
                     Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=set");
                     request.method(HttpMethod.GET);
-
                     ContentResponse response = request.send();
                     assertEquals(HttpServletResponse.SC_OK, response.getStatus());
                     String sessionCookie = response.getHeaders().get("Set-Cookie");
                     assertTrue(sessionCookie != null);
                     // Mangle the cookie, replacing Path with $Path, etc.
                     sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
-
+                    
+                    //ensure request has finished being handled
+                    synchronizer.await(5, TimeUnit.SECONDS);
+                    
                     // Perform a request to server2 using the session cookie from the previous request
                     Request request2 = client.newRequest("http://localhost:" + port2 + contextPath + servletMapping + "?action=get");
                     request2.method(HttpMethod.GET);

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/TestContextScopeListener.java

@@ -21,12 +21,16 @@ package org.eclipse.jetty.server.session;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicReference;
 
+import org.eclipse.jetty.server.HttpChannel.Listener;
+import org.eclipse.jetty.server.HttpChannelState;
+import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.handler.ContextHandler.Context;
 import org.eclipse.jetty.server.handler.ContextHandler.ContextScopeListener;
 
 public class TestContextScopeListener implements ContextScopeListener
 {
     AtomicReference<CountDownLatch> _exitSynchronizer = new AtomicReference<>();
+    boolean listenerAdded = false;
 
     /**
      * @return the exitSynchronizer
@@ -51,9 +55,22 @@ public class TestContextScopeListener implements ContextScopeListener
     }
 
     @Override
-    public void exitScope(Context context, org.eclipse.jetty.server.Request request)
+    public void exitScope(final Context context, final org.eclipse.jetty.server.Request request)
     {
-        if (_exitSynchronizer.get() != null)
-            _exitSynchronizer.get().countDown();
+        if (request != null && !listenerAdded)
+        {
+            listenerAdded=true;
+            request.getHttpChannel().addListener(new Listener()
+            {
+                @Override
+                public void onComplete(Request request)
+                {
+                    Listener.super.onComplete(request);
+                    if (_exitSynchronizer.get() != null)
+                        _exitSynchronizer.get().countDown();
+                }
+                
+            });
+        }
     }
 }

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AsyncTest.java

@@ -20,6 +20,9 @@ package org.eclipse.jetty.server.session;
 
 import java.io.IOException;
 import java.lang.reflect.Proxy;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
 import javax.servlet.AsyncContext;
 import javax.servlet.ServletException;
 import javax.servlet.ServletOutputStream;
@@ -48,15 +51,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  */
 public class AsyncTest
 {
-    public static class LatchServlet extends HttpServlet
-    {
-        @Override
-        protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
-        {
-            resp.getWriter().println("Latched");
-        }
-    }
-
     @Test
     public void testSessionWithAsyncDispatch() throws Exception
     {
@@ -71,12 +65,11 @@ public class AsyncTest
         String mapping = "/server";
 
         ServletContextHandler contextHandler = server.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
         TestServlet servlet = new TestServlet();
         ServletHolder holder = new ServletHolder(servlet);
         contextHandler.addServlet(holder, mapping);
-        LatchServlet latchServlet = new LatchServlet();
-        ServletHolder latchHolder = new ServletHolder(latchServlet);
-        contextHandler.addServlet(latchHolder, "/latch");
 
         server.start();
         int port = server.getPort();
@@ -88,16 +81,17 @@ public class AsyncTest
             String url = "http://localhost:" + port + contextPath + mapping + "?action=async";
 
             //make a request to set up a session on the server
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             ContentResponse response = client.GET(url);
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
 
             String sessionCookie = response.getHeaders().get("Set-Cookie");
             assertTrue(sessionCookie != null);
-
-            //make another request, when this is handled, the first request is definitely finished being handled
-            response = client.GET("http://localhost:" + port + contextPath + "/latch");
-            assertEquals(HttpServletResponse.SC_OK, response.getStatus());
-
+       
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
             //session should now be evicted from the cache after request exited
             String id = TestServer.extractSessionId(sessionCookie);
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
@@ -108,7 +102,6 @@ public class AsyncTest
             server.stop();
         }
     }
-
     @Test
     public void testSessionWithAsyncComplete() throws Exception
     {
@@ -123,12 +116,11 @@ public class AsyncTest
         String mapping = "/server";
 
         ServletContextHandler contextHandler = server.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
         TestServlet servlet = new TestServlet();
         ServletHolder holder = new ServletHolder(servlet);
         contextHandler.addServlet(holder, mapping);
-        LatchServlet latchServlet = new LatchServlet();
-        ServletHolder latchHolder = new ServletHolder(latchServlet);
-        contextHandler.addServlet(latchHolder, "/latch");
 
         server.start();
         int port = server.getPort();
@@ -140,18 +132,19 @@ public class AsyncTest
             String url = "http://localhost:" + port + contextPath + mapping + "?action=asyncComplete";
 
             //make a request to set up a session on the server
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             ContentResponse response = client.GET(url);
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
 
             String sessionCookie = response.getHeaders().get("Set-Cookie");
             assertTrue(sessionCookie != null);
+            String id = TestServer.extractSessionId(sessionCookie);
 
-            //make another request, when this is handled, the first request is definitely finished being handled
-            response = client.GET("http://localhost:" + port + contextPath + "/latch");
-            assertEquals(HttpServletResponse.SC_OK, response.getStatus());
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
 
             //session should now be evicted from the cache after request exited
-            String id = TestServer.extractSessionId(sessionCookie);
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
             assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
         }
@@ -173,6 +166,8 @@ public class AsyncTest
         TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory);
 
         ServletContextHandler contextA = server.addContext("/ctxA");
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextA.addEventListener(scopeListener); //just pick one of the contexts to register the listener
         CrossContextServlet ccServlet = new CrossContextServlet();
         ServletHolder ccHolder = new ServletHolder(ccServlet);
         contextA.addServlet(ccHolder, "/*");
@@ -181,9 +176,7 @@ public class AsyncTest
         TestServlet testServlet = new TestServlet();
         ServletHolder testHolder = new ServletHolder(testServlet);
         contextB.addServlet(testHolder, "/*");
-        LatchServlet latchServlet = new LatchServlet();
-        ServletHolder latchHolder = new ServletHolder(latchServlet);
-        contextB.addServlet(latchHolder, "/latch");
+
 
         server.start();
         int port = server.getPort();
@@ -195,15 +188,16 @@ public class AsyncTest
             String url = "http://localhost:" + port + "/ctxA/test?action=async";
 
             //make a request to set up a session on the server
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             ContentResponse response = client.GET(url);
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
 
             String sessionCookie = response.getHeaders().get("Set-Cookie");
             assertTrue(sessionCookie != null);
 
-            //make another request, when this is handled, the first request is definitely finished being handled
-            response = client.GET("http://localhost:" + port + "/ctxB/latch");
-            assertEquals(HttpServletResponse.SC_OK, response.getStatus());
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
 
             //session should now be evicted from the cache after request exited
             String id = TestServer.extractSessionId(sessionCookie);
@@ -216,6 +210,62 @@ public class AsyncTest
         }
     }
 
+    @Test
+    public void testSessionCreatedBeforeDispatch() throws Exception
+    {
+
+        DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
+        cacheFactory.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
+        SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+        TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory);
+
+        String contextPath = "";
+        String mapping = "/server";
+
+        ServletContextHandler contextHandler = server.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
+
+        TestServlet servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(servlet);
+        contextHandler.addServlet(holder, mapping);
+
+
+        server.start();
+        int port = server.getPort();
+
+        try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
+        {
+            HttpClient client = new HttpClient();
+            client.start();
+            String url = "http://localhost:" + port + contextPath + mapping + "?action=asyncWithSession";
+
+            //make a request to set up a session on the server
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
+            ContentResponse response = client.GET(url);
+            assertEquals(HttpServletResponse.SC_OK, response.getStatus());
+
+            String sessionCookie = response.getHeaders().get("Set-Cookie");
+            assertTrue(sessionCookie != null);
+            String id = TestServer.extractSessionId(sessionCookie);
+            
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+
+            //session should now be evicted from the cache after request exited
+            assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
+            assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
+        }
+        finally
+        {
+            server.stop();
+        }   
+    }
+    
+    
+    
+    
     @Test
     public void testSessionWithCrossContextAsyncComplete() throws Exception
     {
@@ -229,6 +279,8 @@ public class AsyncTest
         TestServer server = new TestServer(0, -1, -1, cacheFactory, storeFactory);
 
         ServletContextHandler contextA = server.addContext("/ctxA");
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextA.addEventListener(scopeListener); //just pick a context
         CrossContextServlet ccServlet = new CrossContextServlet();
         ServletHolder ccHolder = new ServletHolder(ccServlet);
         contextA.addServlet(ccHolder, "/*");
@@ -237,29 +289,28 @@ public class AsyncTest
         TestServlet testServlet = new TestServlet();
         ServletHolder testHolder = new ServletHolder(testServlet);
         contextB.addServlet(testHolder, "/*");
-        LatchServlet latchServlet = new LatchServlet();
-        ServletHolder latchHolder = new ServletHolder(latchServlet);
-        contextB.addServlet(latchHolder, "/latch");
 
         server.start();
         int port = server.getPort();
-
+        HttpClient client = new HttpClient();
+        
         try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
         {
-            HttpClient client = new HttpClient();
+            
             client.start();
             String url = "http://localhost:" + port + "/ctxA/test?action=asyncComplete";
 
             //make a request to set up a session on the server
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             ContentResponse response = client.GET(url);
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
 
             String sessionCookie = response.getHeaders().get("Set-Cookie");
+            
             assertTrue(sessionCookie != null);
-
-            //make another request, when this is handled, the first request is definitely finished being handled
-            response = client.GET("http://localhost:" + port + "/ctxB/latch");
-            assertEquals(HttpServletResponse.SC_OK, response.getStatus());
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
 
             //session should now be evicted from the cache A after request exited
             String id = TestServer.extractSessionId(sessionCookie);
@@ -268,6 +319,7 @@ public class AsyncTest
         }
         finally
         {
+            client.stop();
             server.stop();
         }
     }
@@ -313,6 +365,22 @@ public class AsyncTest
                     response.getWriter().println("OK");
                 }
             }
+            else if ("asyncWithSession".equals(action))
+            {
+                if (request.getAttribute("asyncWithSession") == null)
+                {
+                    request.setAttribute("asyncWithSession", Boolean.TRUE);
+                    AsyncContext acontext = request.startAsync();
+                    HttpSession session = request.getSession(true);
+                    acontext.dispatch();
+                    return;
+                }
+                else
+                {
+                    response.getWriter().println("OK");
+                }
+
+            }
             else if ("asyncComplete".equals(action))
             {
                 AsyncContext acontext = request.startAsync();
@@ -324,7 +392,7 @@ public class AsyncTest
                     {
                         if (out.isReady())
                         {
-                            request.getSession(true);
+                            HttpSession s = request.getSession(true);
                             out.print("OK\n");
                             acontext.complete();
                         }

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/CreationTest.java

@@ -113,10 +113,10 @@ public class CreationTest
             Request request = client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=test");
             response = request.send();
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
-
+   
             //ensure request has finished being handled
             synchronizer.await(5, TimeUnit.SECONDS);
-
+            
             //session should now be evicted from the cache again
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(TestServer.extractSessionId(sessionCookie)));
         }
@@ -406,6 +406,7 @@ public class CreationTest
             if (action != null && action.startsWith("forward"))
             {
                 HttpSession session = request.getSession(true);
+                
                 _id = session.getId();
                 session.setAttribute("value", new Integer(1));
 
@@ -414,7 +415,9 @@ public class CreationTest
                 dispatcherB.forward(request, httpServletResponse);
 
                 if (action.endsWith("inv"))
+                {
                     session.invalidate();
+                }
                 else
                 {
                     session = request.getSession(false);

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DefaultSessionCacheTest.java

@@ -18,25 +18,29 @@
 
 package org.eclipse.jetty.server.session;
 
-import java.util.Collections;
-import java.util.Set;
-import java.util.concurrent.TimeUnit;
-import javax.servlet.http.HttpSessionActivationListener;
-import javax.servlet.http.HttpSessionEvent;
-
-import org.eclipse.jetty.server.Request;
-import org.eclipse.jetty.server.Server;
-import org.eclipse.jetty.server.SessionIdManager;
-import org.eclipse.jetty.servlet.ServletContextHandler;
-import org.junit.jupiter.api.Test;
-
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
+import java.util.Collections;
+import java.util.Random;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import javax.servlet.http.HttpSession;
+import javax.servlet.http.HttpSessionActivationListener;
+import javax.servlet.http.HttpSessionEvent;
+
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.junit.jupiter.api.Test;
 
 /**
  * DefaultSessionCacheTest
@@ -61,90 +65,131 @@ public class DefaultSessionCacheTest
             ++activateCalls;
         }
     }
-
+    
+    
     @Test
-    public void testRenewIdMultipleRequests() throws Exception
+    public void testRenewWithInvalidate() throws Exception
     {
         //Test that invalidation happens on ALL copies of the session that are in-use by requests
-        Server server = new Server();
-
-        SessionIdManager sessionIdManager = new DefaultSessionIdManager(server);
-        server.setSessionIdManager(sessionIdManager);
-        ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS);
-        context.setContextPath("/test");
-        context.setServer(server);
-        context.getSessionHandler().setMaxInactiveInterval((int)TimeUnit.DAYS.toSeconds(1));
-        context.getSessionHandler().setSessionIdManager(sessionIdManager);
+        int inactivePeriod = 20;
+        int scavengePeriod = 3;
 
         DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
         cacheFactory.setSaveOnCreate(true); //ensures that a session is persisted as soon as it is created
+        cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+        TestServer server = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        ServletContextHandler contextHandler = server.addContext("/test");
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
 
-        DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler());
-
-        TestSessionDataStore store = new TestSessionDataStore();
-        cache.setSessionDataStore(store);
-        context.getSessionHandler().setSessionCache(cache);
         TestHttpSessionListener listener = new TestHttpSessionListener();
-        context.getSessionHandler().addEventListener(listener);
+        contextHandler.getSessionHandler().addEventListener(listener);
 
-        server.setHandler(context);
         try
         {
             server.start();
 
-            //create a new session
-            Session s = (Session)context.getSessionHandler().newHttpSession(null);
-            String id = s.getId();
-            context.getSessionHandler().access(s, false); //simulate accessing the request    
-            context.getSessionHandler().complete(s); //simulate completing the request
+            //Make a session
+            Request req0 = new Request(null, null);
+            HttpSession session = contextHandler.getSessionHandler().newHttpSession(req0); //pretend request created session
+            String id = session.getId();
+            req0.onCompleted(); //pretend request exited
 
-            //make 1st request
-            final Session session = context.getSessionHandler().getSession(id); //get the session again
-            assertNotNull(session);
-            context.getSessionHandler().access(session, false); //simulate accessing the request
+            assertTrue(contextHandler.getSessionHandler().getSessionCache().contains(id));
 
-            //make 2nd request
-            final Session session2 = context.getSessionHandler().getSession(id); //get the session again
-            context.getSessionHandler().access(session2, false); //simulate accessing the request
-            assertNotNull(session2);
-            assertTrue(session == session2);
+            //Make a fake request that does not exit the session
+            Request req1 = new Request(null, null);
+            HttpSession s1 = contextHandler.getSessionHandler().getHttpSession(id);
+            assertNotNull(s1);
+            assertSame(session, s1);
+            req1.enterSession(s1);
+            req1.setSessionHandler(contextHandler.getSessionHandler());
+            assertTrue(contextHandler.getSessionHandler().getSessionCache().contains(id));
+            assertEquals(1, ((Session)session).getRequests());
 
-            Thread t2 = new Thread(new Runnable()
+            //Make another fake request that does not exit the session
+            Request req2 = new Request(null, null);
+            HttpSession s2 = contextHandler.getSessionHandler().getHttpSession(id);
+            assertNotNull(s2);
+            assertSame(session, s2);
+            req2.enterSession(s2);
+            req2.setSessionHandler(contextHandler.getSessionHandler());
+            assertEquals(2, ((Session)session).getRequests());
+
+            //Renew the session id
+            Request req3 = new Request(null, null);
+            final HttpSession s3 = contextHandler.getSessionHandler().getHttpSession(id);
+            assertNotNull(s3);
+            assertSame(session, s3);
+            req3.enterSession(s3);
+            req3.setSessionHandler(contextHandler.getSessionHandler());
+
+            //Invalidate the session
+            Request req4 = new Request(null, null);
+            final HttpSession s4 = contextHandler.getSessionHandler().getHttpSession(id);
+            assertNotNull(s4);
+            assertSame(session, s4);
+            req4.enterSession(s4);
+            req4.setSessionHandler(contextHandler.getSessionHandler());
+
+            Thread renewThread = new Thread(new Runnable()
             {
                 @Override
                 public void run()
                 {
-                    System.err.println("Starting session id renewal");
-                    session2.renewId(new Request(null, null));
-                    System.err.println("Finished session id renewal");
-                }
-            });
-            t2.start();
+                    //simulate req3 calling Request.changeSessionId
+                    String oldid = ((Session)s3).getId(); //old id
 
-            Thread t = new Thread(new Runnable()
-            {
-
-                @Override
-                public void run()
-                {
-                    System.err.println("Starting invalidation");
+                    //Session may already be invalid depending on timing
                     try
                     {
-                        Thread.sleep(1000L);
+                        ((Session)s3).renewId(req3);
+                        //After this call, the session must have changed id, and it may also be
+                        //invalid, depending on timing.
+                        assertFalse(oldid.equals(((Session)s3).getId()));
                     }
-                    catch (Exception e)
+                    catch (IllegalStateException e)
                     {
-                        e.printStackTrace();
+                        //the session was invalid before we called renewId
+                    }
+                    catch (Throwable e)
+                    {
+                        //anything else is a failure
+                        fail(e);
+                    }
+                }
+            }
+                );
+
+            Thread invalidateThread = new Thread(new Runnable()
+            {
+            
+                @Override
+                public void run()
+                {
+                    //simulate req4 doing an invalidate that we hope overlaps with req3 renewId
+                    try
+                    {
+                        Random random = new Random();
+                        if ((random.nextInt(10) % 2)  == 0)
+                            Thread.currentThread().sleep(2); //small sleep to try and make timing more random
+                        ((Session)s4).invalidate();
+                        assertFalse(((Session)s4).isValid());
+                    }
+                    catch (InterruptedException e)
+                    {
+                        
                     }
-                    session.invalidate();
-                    System.err.println("Finished invalidation");
                 }
             }
             );
-            t.start();
 
-            t.join();
-            t2.join();
+            invalidateThread.start();
+            renewThread.start();
+            renewThread.join();
+            invalidateThread.join();
+
         }
         finally
         {
@@ -180,10 +225,10 @@ public class DefaultSessionCacheTest
         SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
         Session session = cache.newSession(data);
         TestSessionActivationListener listener = new TestSessionActivationListener();
-        cache.put("1234", session);
+        cache.add("1234", session);
         assertTrue(cache.contains("1234"));
         session.setAttribute("aaa", listener);
-        cache.put("1234", session);
+        cache.release("1234", session);
 
         assertTrue(store.exists("1234"));
         assertTrue(cache.contains("1234"));
@@ -255,7 +300,7 @@ public class DefaultSessionCacheTest
         long now = System.currentTimeMillis();
         SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
         Session session = cache.newSession(data);
-        cache.put("1234", session);
+        cache.add("1234", session);
         assertTrue(cache.contains("1234"));
 
         cache.renewSessionId("1234", "5678", "1234.foo", "5678.foo");
@@ -293,10 +338,11 @@ public class DefaultSessionCacheTest
         SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
         Session session = cache.newSession(data);
 
-        //put the session in the cache
-        cache.put("1234", session);
+        //ensure the session is in the cache
+        cache.add("1234", session);
 
-        assertNotNull(cache.get("1234"));
+        //peek into the cache and see if it is there
+        assertTrue(((AbstractSessionCache)cache).contains("1234"));
     }
 
     /**
@@ -329,13 +375,14 @@ public class DefaultSessionCacheTest
         assertFalse(cache.contains("1234"));
 
         Session session = cache.get("1234");
+        assertEquals(1, session.getRequests());
         assertNotNull(session);
         assertEquals("1234", session.getId());
         assertEquals(now - 20, session.getCreationTime());
     }
 
     @Test
-    public void testPutRequestsStillActive()
+    public void testAdd()
         throws Exception
     {
         Server server = new Server();
@@ -353,20 +400,23 @@ public class DefaultSessionCacheTest
         context.getSessionHandler().setSessionCache(cache);
         context.start();
 
-        //make a session
+        //add data for a session to the store
         long now = System.currentTimeMillis();
         SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
         data.setExpiry(now + TimeUnit.DAYS.toMillis(1));
+        
+        //create a session for the existing session data, add it to the cache
         Session session = cache.newSession(data);
-        session.access(now); //simulate request still active
-        cache.put("1234", session);
+        cache.add("1234", session);
+
+        assertEquals(1, session.getRequests());
         assertTrue(session.isResident());
         assertTrue(cache.contains("1234"));
         assertFalse(store.exists("1234"));
     }
 
     @Test
-    public void testPutLastRequest()
+    public void testRelease()
         throws Exception
     {
         Server server = new Server();
@@ -384,14 +434,19 @@ public class DefaultSessionCacheTest
         context.getSessionHandler().setSessionCache(cache);
         context.start();
 
-        //make a session
+        //create data for a session in the store
         long now = System.currentTimeMillis();
         SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
         data.setExpiry(now + TimeUnit.DAYS.toMillis(1));
+        
+        //make a session for the existing id, add to cache
         Session session = cache.newSession(data);
-        session.access(now); //simulate request still active
-        session.complete(); //simulate request exiting
-        cache.put("1234", session);
+        cache.add("1234", session);
+        
+        //release use of newly added session
+        cache.release("1234", session);
+
+        assertEquals(0, session.getRequests());
         assertTrue(session.isResident());
         assertTrue(cache.contains("1234"));
         assertTrue(store.exists("1234"));
@@ -426,9 +481,7 @@ public class DefaultSessionCacheTest
         long now = System.currentTimeMillis();
         SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
         Session session = cache.newSession(data);
-
-        //put the session in the cache
-        cache.put("1234", session);
+        cache.add("1234", session);
         assertTrue(cache.contains("1234"));
     }
 
@@ -465,7 +518,7 @@ public class DefaultSessionCacheTest
 
         //test one that exists in the cache also
         Session session = cache.newSession(data);
-        cache.put("1234", session);
+        cache.add("1234", session);
         assertTrue(cache.exists("1234"));
     }
 
@@ -484,6 +537,7 @@ public class DefaultSessionCacheTest
 
         DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
         cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        cacheFactory.setSaveOnCreate(true);
         DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler());
 
         TestSessionDataStore store = new TestSessionDataStore();
@@ -505,9 +559,8 @@ public class DefaultSessionCacheTest
         assertFalse(cache.contains("1234"));
 
         //test remove of session in both store and cache
-        data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
-        session = cache.newSession(data);
-        cache.put("1234", session);
+        session = cache.newSession(null, "1234",now - 20 ,TimeUnit.MINUTES.toMillis(10));//saveOnCreate ensures write to store
+        cache.add("1234", session);
         assertTrue(store.exists("1234"));
         assertTrue(cache.contains("1234"));
         session = cache.delete("1234");
@@ -544,14 +597,14 @@ public class DefaultSessionCacheTest
         SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
         data.setExpiry(now + TimeUnit.DAYS.toMillis(1));
         Session session = cache.newSession(data);
-        cache.put("1234", session);
+        cache.add("1234", session);
+        cache.release("1234", session);
         assertTrue(cache.exists("1234"));
         result = cache.checkExpiration(Collections.singleton("1234"));
         assertTrue(result.isEmpty());
 
         //test candidates that are in the cache AND expired
         data.setExpiry(1);
-        cache.put("1234", session);
         result = cache.checkExpiration(Collections.singleton("1234"));
         assertEquals(1, result.size());
         assertEquals("1234", result.iterator().next());
@@ -597,7 +650,8 @@ public class DefaultSessionCacheTest
         //ie nothing happens to the session
 
         //test session that is resident but not valid
-        cache.put("1234", session);
+        cache.add("1234", session);
+        cache.release("1234", session); //this will write session
         session._state = Session.State.INVALID;
         cache.checkInactiveSession(session);
         assertTrue(store.exists("1234"));
@@ -627,7 +681,7 @@ public class DefaultSessionCacheTest
         SessionData data2 = store.newSessionData("567", now, now - TimeUnit.SECONDS.toMillis(30), now - TimeUnit.SECONDS.toMillis(40), TimeUnit.MINUTES.toMillis(10));
         data2.setExpiry(now + TimeUnit.DAYS.toMillis(1));//not expired
         Session session2 = cache.newSession(data2);
-        cache.put("567", session2);//ensure session is in cache
+        cache.add("567", session2);//ensure session is in cache
         cache.setEvictionPolicy(SessionCache.EVICT_ON_SESSION_EXIT);
         session2.access(System.currentTimeMillis());//simulate 1 request in session
         assertTrue(cache.contains("567"));
@@ -636,7 +690,7 @@ public class DefaultSessionCacheTest
 
         //test  EVICT_ON_SESSION_EXIT - requests not active
         //this should not affect the session because this is an idle test only
-        session2.complete(); //simulate last request leaving session
+        session2.complete(); //NOTE:don't call cache.release as this will remove the session
         cache.checkInactiveSession(session2);
         assertTrue(cache.contains("567"));
     }
@@ -665,10 +719,13 @@ public class DefaultSessionCacheTest
         SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
         data.setExpiry(now + TimeUnit.DAYS.toMillis(1));
         Session session = cache.newSession(data);
-        cache.put("1234", session); //make it resident
+        cache.add("1234", session); //make it resident
         assertTrue(cache.contains("1234"));
         long accessed = now - TimeUnit.SECONDS.toMillis(30); //make it idle
         data.setAccessed(accessed);
+        cache.release("1234", session);
+        assertTrue(cache.contains("1234"));
+        assertTrue(session.isResident());
         cache.checkInactiveSession(session);
         assertFalse(cache.contains("1234"));
         assertFalse(session.isResident());

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DirtyAttributeTest.java

@@ -106,14 +106,19 @@ public class DirtyAttributeTest
             try
             {
                 // Perform a request to create a session
+                CountDownLatch latch = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(latch);
                 ContentResponse response = client.GET("http://localhost:" + port + "/mod/test?action=create");
 
                 assertEquals(HttpServletResponse.SC_OK, response.getStatus());
                 String sessionCookie = response.getHeaders().get("Set-Cookie");
                 assertTrue(sessionCookie != null);
+                
+                //ensure request finished
+                latch.await(5, TimeUnit.SECONDS);
 
                 //do another request to change the session attribute
-                CountDownLatch latch = new CountDownLatch(1);
+                latch = new CountDownLatch(1);
                 scopeListener.setExitSynchronizer(latch);
                 Request request = client.newRequest("http://localhost:" + port + "/mod/test?action=setA");
                 response = request.send();
@@ -126,6 +131,8 @@ public class DirtyAttributeTest
                 A_VALUE.assertActivatesEquals(1);
                 A_VALUE.assertBindsEquals(1);
                 A_VALUE.assertUnbindsEquals(0);
+                
+
 
                 //do another request using the cookie to try changing the session attribute to the same value again  
                 latch = new CountDownLatch(1);

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/IdleSessionTest.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.server.session;
 
 import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
@@ -75,6 +76,8 @@ public class IdleSessionTest
         _server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
         ServletHolder holder = new ServletHolder(_servlet);
         ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
         contextHandler.addServlet(holder, servletMapping);
         _server1.start();
         int port1 = _server1.getPort();
@@ -86,24 +89,34 @@ public class IdleSessionTest
             String url = "http://localhost:" + port1 + contextPath + servletMapping;
 
             //make a request to set up a session on the server
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             ContentResponse response = client.GET(url + "?action=init");
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
             String sessionCookie = response.getHeaders().get("Set-Cookie");
             assertTrue(sessionCookie != null);
+            
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
 
             //and wait until the session should be passivated out
             pause(evictionSec * 2);
-
+            
             //check that the session has been idled
             String id = TestServer.extractSessionId(sessionCookie);
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
             assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
 
             //make another request to reactivate the session
+            synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             Request request = client.newRequest(url + "?action=test");
             ContentResponse response2 = request.send();
             assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
-
+            
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
             //check session reactivated
             assertTrue(contextHandler.getSessionHandler().getSessionCache().contains(id));
 
@@ -119,17 +132,27 @@ public class IdleSessionTest
             ((TestSessionDataStore)contextHandler.getSessionHandler().getSessionCache().getSessionDataStore())._map.clear();
 
             //make a request
+            synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             request = client.newRequest(url + "?action=testfail");
             response2 = request.send();
             assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
 
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
             //Test trying to reactivate an expired session (ie before the scavenger can get to it)
             //make a request to set up a session on the server
+            synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             response = client.GET(url + "?action=init");
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
             sessionCookie = response.getHeaders().get("Set-Cookie");
             assertTrue(sessionCookie != null);
             id = TestServer.extractSessionId(sessionCookie);
+            
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
 
             //and wait until the session should be idled out
             pause(evictionSec * 2);
@@ -146,9 +169,14 @@ public class IdleSessionTest
             pause(inactivePeriod + (3 * scavengePeriod));
 
             //make another request to reactivate the session
+            synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             request = client.newRequest(url + "?action=testfail");
             response2 = request.send();
             assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
+            
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
 
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
             assertFalse(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
@@ -174,6 +202,8 @@ public class IdleSessionTest
         _server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
         ServletHolder holder = new ServletHolder(_servlet);
         ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
         contextHandler.addServlet(holder, servletMapping);
         _server1.start();
         int port1 = _server1.getPort();
@@ -185,10 +215,15 @@ public class IdleSessionTest
             String url = "http://localhost:" + port1 + contextPath + servletMapping;
 
             //make a request to set up a session on the server
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             ContentResponse response = client.GET(url + "?action=init");
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
             String sessionCookie = response.getHeaders().get("Set-Cookie");
             assertTrue(sessionCookie != null);
+            
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
 
             //the session should never be cached
             String id = TestServer.extractSessionId(sessionCookie);
@@ -196,10 +231,15 @@ public class IdleSessionTest
             assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
 
             //make another request to reactivate the session
+            synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             Request request = client.newRequest(url + "?action=test");
             ContentResponse response2 = request.send();
             assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
-
+            
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
             //check session still not in the cache
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
             assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
@@ -209,17 +249,28 @@ public class IdleSessionTest
             ((TestSessionDataStore)contextHandler.getSessionHandler().getSessionCache().getSessionDataStore())._map.clear();
 
             //make a request
+            synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             request = client.newRequest(url + "?action=testfail");
             response2 = request.send();
             assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
 
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
             //Test trying to reactivate an expired session (ie before the scavenger can get to it)
             //make a request to set up a session on the server
+            synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             response = client.GET(url + "?action=init");
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
             sessionCookie = response.getHeaders().get("Set-Cookie");
             assertTrue(sessionCookie != null);
             id = TestServer.extractSessionId(sessionCookie);
+            
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
 
             //stop the scavenger
             if (_server1.getHouseKeeper() != null)
@@ -233,10 +284,15 @@ public class IdleSessionTest
             pause(inactivePeriod + (3 * scavengePeriod));
 
             //make another request to reactivate the session
+            synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             request = client.newRequest(url + "?action=testfail");
             response2 = request.send();
             assertEquals(HttpServletResponse.SC_OK, response2.getStatus());
-
+            
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
             assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
             assertFalse(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
         }

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/NullSessionCacheTest.java

@@ -55,15 +55,18 @@ public class NullSessionCacheTest
         SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
         data.setExpiry(now + TimeUnit.DAYS.toMillis(1));
         Session session = cache.newSession(null, data); //mimic a request making a session
-        session.complete(); //mimic request leaving session
-        cache.put("1234", session); //null cache doesn't store the session
-        assertFalse(cache.contains("1234"));
+        cache.add("1234", session); 
+        assertFalse(cache.contains("1234"));//null cache doesn't actually store the session
+        
+        //mimic releasing the session after the request is finished
+        cache.release("1234", session);
         assertTrue(store.exists("1234"));
+        assertFalse(cache.contains("1234"));
 
+        //simulate a new request using the previously created session
         session = cache.get("1234"); //get the session again
         session.access(now); //simulate a request
-        session.complete(); //simulate a request leaving
-        cache.put("1234", session); //finish with the session
+        cache.release("1234", session); //finish with the session
 
         assertFalse(session.isResident());
     }

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SaveOptimizeTest.java

@@ -124,6 +124,8 @@ public class SaveOptimizeTest
         _servlet = new TestServlet();
         ServletHolder holder = new ServletHolder(_servlet);
         ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
         contextHandler.addServlet(holder, servletMapping);
         _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
         _server1.start();
@@ -138,11 +140,16 @@ public class SaveOptimizeTest
                 String url = "http://localhost:" + port1 + contextPath + servletMapping + "?action=create&check=true";
 
                 //make a request to set up a session on the server
+                CountDownLatch synchronizer = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(synchronizer);
                 ContentResponse response = client.GET(url);
                 assertEquals(HttpServletResponse.SC_OK, response.getStatus());
                 String sessionCookie = response.getHeaders().get("Set-Cookie");
                 assertNotNull(sessionCookie);
                 String sessionId = TestServer.extractSessionId(sessionCookie);
+                
+                //ensure request has finished being handled
+                synchronizer.await(5, TimeUnit.SECONDS);
 
                 SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                 assertNotNull(data);
@@ -151,9 +158,14 @@ public class SaveOptimizeTest
                 int initialNumSaves = getNumSaves();
                 for (int i = 0; i < 5; i++)
                 {
-                    // Perform a request to contextB with the same session cookie
+                    // Perform a request with the same session cookie
+                    synchronizer = new CountDownLatch(1);
+                    scopeListener.setExitSynchronizer(synchronizer);
                     client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=noop").send();
 
+                    //ensure request has finished being handled
+                    synchronizer.await(5, TimeUnit.SECONDS);
+                    
                     //check session is unchanged
                     SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                     assertNotNull(d);
@@ -195,6 +207,8 @@ public class SaveOptimizeTest
         _servlet = new TestServlet();
         ServletHolder holder = new ServletHolder(_servlet);
         ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
         contextHandler.addServlet(holder, servletMapping);
         _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
         _server1.start();
@@ -209,19 +223,29 @@ public class SaveOptimizeTest
                 String url = "http://localhost:" + port1 + contextPath + servletMapping + "?action=create&check=true";
 
                 //make a request to set up a session on the server
+                CountDownLatch synchronizer = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(synchronizer);
                 ContentResponse response = client.GET(url);
                 assertEquals(HttpServletResponse.SC_OK, response.getStatus());
                 String sessionCookie = response.getHeaders().get("Set-Cookie");
                 assertNotNull(sessionCookie);
                 String sessionId = TestServer.extractSessionId(sessionCookie);
 
+                //ensure request has finished being handled
+                synchronizer.await(5, TimeUnit.SECONDS);
+                
                 SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                 assertNotNull(data);
 
                 // Perform a request to do nothing with the same session cookie
                 int numSavesBefore = getNumSaves();
+                synchronizer = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(synchronizer);
                 client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=noop").send();
-
+                
+                //ensure request has finished being handled
+                synchronizer.await(5, TimeUnit.SECONDS);
+                
                 //check session not saved
                 SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                 assertNotNull(d);
@@ -229,8 +253,13 @@ public class SaveOptimizeTest
 
                 // Perform a request to mutate the session
                 numSavesBefore = getNumSaves();
+                synchronizer = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(synchronizer);
                 client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=mutate").send();
 
+                //ensure request has finished being handled
+                synchronizer.await(5, TimeUnit.SECONDS);
+                
                 //check session is saved
                 d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                 assertNotNull(d);
@@ -268,6 +297,8 @@ public class SaveOptimizeTest
         _servlet = new TestServlet();
         ServletHolder holder = new ServletHolder(_servlet);
         ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
         contextHandler.addServlet(holder, servletMapping);
         _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
         _server1.start();
@@ -282,12 +313,17 @@ public class SaveOptimizeTest
                 String url = "http://localhost:" + port1 + contextPath + servletMapping + "?action=create&check=true";
 
                 //make a request to set up a session on the server
+                CountDownLatch synchronizer = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(synchronizer);
                 ContentResponse response = client.GET(url);
                 assertEquals(HttpServletResponse.SC_OK, response.getStatus());
                 String sessionCookie = response.getHeaders().get("Set-Cookie");
                 assertNotNull(sessionCookie);
                 String sessionId = TestServer.extractSessionId(sessionCookie);
 
+                //ensure request has finished being handled
+                synchronizer.await(5, TimeUnit.SECONDS);
+                
                 SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                 assertNotNull(data);
                 long lastSaved = data.getLastSaved();
@@ -295,8 +331,13 @@ public class SaveOptimizeTest
                 //make another request, session should not change
 
                 // Perform a request to do nothing with the same session cookie
+                synchronizer = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(synchronizer);
                 client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=noop").send();
-
+               
+                //ensure request has finished being handled
+                synchronizer.await(5, TimeUnit.SECONDS);
+                
                 //check session not saved
                 SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                 assertNotNull(d);
@@ -306,8 +347,13 @@ public class SaveOptimizeTest
                 Thread.sleep(1000 * savePeriod);
 
                 // Perform a request to do nothing with the same session cookie
+                synchronizer = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(synchronizer);
                 client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=noop").send();
-
+                
+                //ensure request has finished being handled
+                synchronizer.await(5, TimeUnit.SECONDS);
+                
                 //check session is saved
                 d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                 assertNotNull(d);
@@ -346,6 +392,8 @@ public class SaveOptimizeTest
         _servlet = new TestServlet();
         ServletHolder holder = new ServletHolder(_servlet);
         ServletContextHandler contextHandler = _server1.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
         contextHandler.addServlet(holder, servletMapping);
         _servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
         _server1.start();
@@ -360,11 +408,16 @@ public class SaveOptimizeTest
                 String url = "http://localhost:" + port1 + contextPath + servletMapping + "?action=create&check=true";
 
                 //make a request to set up a session on the server
+                CountDownLatch synchronizer = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(synchronizer);
                 ContentResponse response = client.GET(url);
                 assertEquals(HttpServletResponse.SC_OK, response.getStatus());
                 String sessionCookie = response.getHeaders().get("Set-Cookie");
                 assertNotNull(sessionCookie);
                 String sessionId = TestServer.extractSessionId(sessionCookie);
+                
+                //ensure request has finished being handled
+                synchronizer.await(5, TimeUnit.SECONDS);
 
                 SessionData data = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                 assertNotNull(data);
@@ -372,8 +425,13 @@ public class SaveOptimizeTest
                 assertTrue(lastSaved > 0); //check session created was saved
 
                 // Perform a request to do nothing with the same session cookie, check the session object is different
+                synchronizer = new CountDownLatch(1);
+                scopeListener.setExitSynchronizer(synchronizer);
                 client.newRequest("http://localhost:" + port1 + contextPath + servletMapping + "?action=noop&check=diff").send();
 
+                //ensure request has finished being handled
+                synchronizer.await(5, TimeUnit.SECONDS);
+                
                 //check session not saved
                 SessionData d = contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().load(sessionId);
                 assertNotNull(d);

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionRenewTest.java

@@ -19,6 +19,10 @@
 package org.eclipse.jetty.server.session;
 
 import java.io.IOException;
+import java.net.HttpCookie;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
@@ -60,7 +64,19 @@ public class SessionRenewTest
 
         //make the server with a NullSessionCache
         _server = new TestServer(0, -1, -1, cacheFactory, storeFactory);
-        doTest(new RenewalVerifier());
+        doTest(new RenewalVerifier()
+            {
+
+                @Override
+                public void verify(WebAppContext context, String oldSessionId, String newSessionId) throws Exception
+                {
+                    //null cache means it should contain neither session
+                    assertFalse(context.getSessionHandler().getSessionCache().contains(newSessionId));
+                    assertFalse(context.getSessionHandler().getSessionCache().contains(oldSessionId));
+                    super.verify(context, oldSessionId, newSessionId);
+                }
+            
+            });
     }
 
     /**
@@ -90,6 +106,73 @@ public class SessionRenewTest
         });
     }
 
+    @Test
+    public void testSessionRenewalMultiContext() throws Exception
+    {
+        DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
+        cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+
+        _server = new TestServer(0, -1, -1, cacheFactory, storeFactory);
+        
+        String contextPathA = "";
+        String servletMapping = "/server";
+        WebAppContext contextA = _server.addWebAppContext(".", contextPathA);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextA.addEventListener(scopeListener);
+        contextA.setParentLoaderPriority(true);
+        contextA.addServlet(TestServlet.class, servletMapping);
+        
+        WebAppContext contextB = _server.addWebAppContext(".", "/B");
+        contextB.setParentLoaderPriority(true);
+
+        HttpClient client = new HttpClient();
+        try
+        {
+            _server.start();
+            int port = _server.getPort();
+
+            client.start();
+
+            //pre-create session data for both contextA and contextB
+            long now = System.currentTimeMillis();
+            SessionData dataA = contextA.getSessionHandler().getSessionCache().getSessionDataStore().newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
+            contextA.getSessionHandler().getSessionCache().getSessionDataStore().store("1234", dataA);
+            SessionData dataB = contextB.getSessionHandler().getSessionCache().getSessionDataStore().newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10));
+            contextB.getSessionHandler().getSessionCache().getSessionDataStore().store("1234", dataB);
+
+            //make a request to change the sessionid
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
+            Request request = client.newRequest("http://localhost:" + port + contextPathA + servletMapping + "?action=renew");
+            request.cookie(new HttpCookie(SessionHandler.__DefaultSessionCookie, "1234"));
+            ContentResponse renewResponse = request.send();
+            assertEquals(HttpServletResponse.SC_OK, renewResponse.getStatus());
+            String newSessionCookie = renewResponse.getHeaders().get("Set-Cookie");
+            assertTrue(newSessionCookie != null);
+            String updatedId = TestServer.extractSessionId(newSessionCookie);
+
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
+            //session ids should be updated on all contexts
+            contextA.getSessionHandler().getSessionCache().contains(updatedId);
+            contextB.getSessionHandler().getSessionCache().contains(updatedId);
+
+            Session sessiona = ((AbstractSessionCache)contextA.getSessionHandler().getSessionCache()).getAndEnter(updatedId, false);
+            Session sessionb = ((AbstractSessionCache)contextB.getSessionHandler().getSessionCache()).getAndEnter(updatedId, false);
+
+            //sessions should nor have any usecounts
+            assertEquals(0, sessiona.getRequests());
+            assertEquals(0, sessionb.getRequests());
+
+        }
+        finally
+        {
+            client.stop();
+            _server.stop();
+        }
+    }
     /**
      * Perform the test by making a request to create a session
      * then another request that will renew the session id.
@@ -101,6 +184,8 @@ public class SessionRenewTest
         String contextPath = "";
         String servletMapping = "/server";
         WebAppContext context = _server.addWebAppContext(".", contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        context.addEventListener(scopeListener);
         context.setParentLoaderPriority(true);
         context.addServlet(TestServlet.class, servletMapping);
         TestHttpSessionIdListener testListener = new TestHttpSessionIdListener();
@@ -115,18 +200,28 @@ public class SessionRenewTest
             client.start();
 
             //make a request to create a session
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             ContentResponse response = client.GET("http://localhost:" + port + contextPath + servletMapping + "?action=create");
             assertEquals(HttpServletResponse.SC_OK, response.getStatus());
 
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
             String sessionCookie = response.getHeaders().get("Set-Cookie");
             assertTrue(sessionCookie != null);
             assertFalse(testListener.isCalled());
 
             //make a request to change the sessionid
+            synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
             Request request = client.newRequest("http://localhost:" + port + contextPath + servletMapping + "?action=renew");
             ContentResponse renewResponse = request.send();
-
             assertEquals(HttpServletResponse.SC_OK, renewResponse.getStatus());
+           
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+            
             String renewSessionCookie = renewResponse.getHeaders().get("Set-Cookie");
             assertNotNull(renewSessionCookie);
             assertNotSame(sessionCookie, renewSessionCookie);
@@ -175,6 +270,16 @@ public class SessionRenewTest
         }
     }
 
+    public static class TestServletB extends HttpServlet
+    {
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
+        {
+           //Ensure a session exists
+        }
+        
+    }
     public static class TestServlet extends HttpServlet
     {
         private static final long serialVersionUID = 1L;
@@ -194,7 +299,8 @@ public class SessionRenewTest
                 assertTrue(beforeSession != null);
                 String beforeSessionId = beforeSession.getId();
 
-                ((Session)beforeSession).renewId(request);
+                //((Session)beforeSession).renewId(request);
+                request.changeSessionId();
 
                 HttpSession afterSession = request.getSession(false);
 
@@ -210,10 +316,6 @@ public class SessionRenewTest
                 assertTrue(sessionIdManager.isIdInUse(afterSessionId)); //new session id should be in use
                 assertFalse(sessionIdManager.isIdInUse(beforeSessionId));
 
-                HttpSession session = sessionManager.getSession(afterSessionId);
-                assertNotNull(session);
-                session = sessionManager.getSession(beforeSessionId);
-                assertNull(session);
 
                 if (((Session)afterSession).isIdChanged())
                     ((org.eclipse.jetty.server.Response)response).replaceCookie(sessionManager.getSessionCookie(afterSession, request.getContextPath(), request.isSecure()));