Issue #2427 Stop and start session idle timer (#2466)

* Issue #2427 Stop and start session idle timer

Signed-off-by: Jan Bartel <janb@webtide.com>
This commit is contained in:
Jan Bartel 2018-05-03 08:05:26 +10:00 committed by GitHub
parent 1f835b9f23
commit 5515e13649
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 272 additions and 121 deletions

View File

@ -158,6 +158,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
/**
* PlaceHolder
*/
@ -165,11 +166,12 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
{
/**
* @param handler SessionHandler to which this session belongs
* @param data the session data
*/
public PlaceHolderSession(SessionData data)
public PlaceHolderSession(SessionHandler handler, SessionData data)
{
super(null, data);
super(handler, data);
}
}
@ -351,7 +353,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
LOG.debug("Session {} not found locally, attempting to load", id);
//didn't get a session, try and create one and put in a placeholder for it
PlaceHolderSession phs = new PlaceHolderSession (new SessionData(id, null, null,0,0,0,0));
PlaceHolderSession phs = new PlaceHolderSession (_handler, new SessionData(id, null, null,0,0,0,0));
Lock phsLock = phs.lock();
Session s = doPutIfAbsent(id, phs);
if (s == null)
@ -385,7 +387,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
{
//successfully swapped in the session
session.setResident(true);
session.updateInactivityTimer();
phsLock.close();
break;
}
@ -506,15 +507,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
if (!session.isValid())
return;
if (_sessionDataStore == null)
{
if (LOG.isDebugEnabled()) LOG.debug("No SessionDataStore, putting into SessionCache only id={}", id);
session.setResident(true);
if (doPutIfAbsent(id, session) == null) //ensure it is in our map
session.updateInactivityTimer();
return;
}
//don't do anything with the session until the last request for it has finished
if ((session.getRequests() <= 0))
{
@ -533,8 +525,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
else
{
session.setResident(true);
if (doPutIfAbsent(id,session) == null) //ensure it is in our map
session.updateInactivityTimer();
doPutIfAbsent(id,session); //ensure it is in our map
if (LOG.isDebugEnabled())LOG.debug("Non passivating SessionDataStore, session in SessionCache only id={}",id);
}
}
@ -557,8 +548,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
//reactivate the session
session.didActivate();
session.setResident(true);
if (doPutIfAbsent(id,session) == null) //ensure it is in our map
session.updateInactivityTimer();
doPutIfAbsent(id,session);//ensure it is in our map
if (LOG.isDebugEnabled())LOG.debug("Session reactivated id={}",id);
}
}
@ -567,8 +557,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
{
if (LOG.isDebugEnabled()) LOG.debug("Req count={} for id={}",session.getRequests(),id);
session.setResident(true);
if (doPutIfAbsent(id, session) == null) //ensure it is the map, but don't save it to the backing store until the last request exists
session.updateInactivityTimer();
doPutIfAbsent(id, session); //ensure it is the map, but don't save it to the backing store until the last request exists
}
}
}
@ -639,7 +628,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
//delete it from the session object store
if (session != null)
{
session.stopInactivityTimer();
session.setResident(false);
}
@ -729,7 +717,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
catch (Exception e)
{
LOG.warn("Passivation of idle session {} failed", session.getId(), e);
session.updateInactivityTimer();
//session.updateInactivityTimer();
}
}
}

View File

@ -166,6 +166,7 @@ public class DefaultSessionCache extends AbstractSessionCache
LOG.warn(e);
}
doDelete (session.getId()); //remove from memory
session.setResident(false);
}
else
{
@ -223,5 +224,4 @@ public class DefaultSessionCache extends AbstractSessionCache
return result;
}
}

View File

@ -113,7 +113,4 @@ public class NullSessionCache extends AbstractSessionCache
{
LOG.warn("Ignoring eviction setting:"+evictionTimeout);
}
}

View File

@ -25,7 +25,6 @@ import java.util.Enumeration;
import java.util.Iterator;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import javax.servlet.ServletContext;
import javax.servlet.http.HttpServletRequest;
@ -35,7 +34,7 @@ import javax.servlet.http.HttpSessionBindingListener;
import javax.servlet.http.HttpSessionContext;
import javax.servlet.http.HttpSessionEvent;
import org.eclipse.jetty.io.IdleTimeout;
import org.eclipse.jetty.io.CyclicTimeout;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.thread.Locker;
@ -82,8 +81,8 @@ public class Session implements SessionHandler.SessionIf
protected SessionData _sessionData; //the actual data associated with a session
protected SessionHandler _handler; //the manager of the session
protected final SessionData _sessionData; //the actual data associated with a session
protected final SessionHandler _handler; //the manager of the session
protected String _extendedId; //the _id plus the worker name
protected long _requests;
protected boolean _idChanged;
@ -91,68 +90,77 @@ public class Session implements SessionHandler.SessionIf
protected State _state = State.VALID; //state of the session:valid,invalid or being invalidated
protected Locker _lock = new Locker(); //sync lock
protected boolean _resident = false;
protected SessionInactivityTimeout _sessionInactivityTimer = null;
protected final SessionInactivityTimer _sessionInactivityTimer;
/* ------------------------------------------------------------- */
/**
* SessionInactivityTimeout
* SessionInactivityTimer
*
* Each Session has a timer associated with it that fires whenever
* it has been idle (ie not referenced by a request) for a
* it has been idle (ie not accessed by a request) for a
* configurable amount of time, or the Session expires.
*
* @see SessionCache
*
*/
public class SessionInactivityTimeout extends IdleTimeout
public class SessionInactivityTimer
{
/**
*
*/
public SessionInactivityTimeout()
protected final CyclicTimeout _timer;
protected long _msec = -1;
public SessionInactivityTimer()
{
super(getSessionHandler().getScheduler());
_timer = new CyclicTimeout((getSessionHandler().getScheduler()))
{
@Override
public void onTimeoutExpired()
{
if (LOG.isDebugEnabled()) LOG.debug("Timer expired for session {}", getId());
getSessionHandler().sessionInactivityTimerExpired(Session.this);
}
};
}
/**
* @see org.eclipse.jetty.io.IdleTimeout#onIdleExpired(java.util.concurrent.TimeoutException)
*/
@Override
protected void onIdleExpired(TimeoutException timeout)
{
//called when the timer goes off
if (LOG.isDebugEnabled()) LOG.debug("Timer expired for session {}", getId());
getSessionHandler().sessionInactivityTimerExpired(Session.this);
}
/**
* @see org.eclipse.jetty.io.IdleTimeout#isOpen()
* @param ms the timeout to set; -1 means that the timer will not be scheduled
*/
@Override
public boolean isOpen()
public void setTimeout(long ms)
{
// Called to determine if the timer should be reset
// True if:
// 1. the session is still valid
// BUT if passivated out to disk, do we really want this timer to keep going off?
try (Lock lock = _lock.lock())
_msec = ms;
if (LOG.isDebugEnabled()) LOG.debug("Session {} timer={}ms",getId(), ms);
}
public void schedule ()
{
if (_msec > 0)
{
return isValid() && isResident();
if (LOG.isDebugEnabled()) LOG.debug("(Re)starting timer for session {} at {}ms",getId(), _msec);
_timer.schedule(_msec, TimeUnit.MILLISECONDS);
}
else
{
if (LOG.isDebugEnabled()) LOG.debug("Not starting timer for session {}",getId());
}
}
/**
* @see org.eclipse.jetty.io.IdleTimeout#setIdleTimeout(long)
*/
@Override
public void setIdleTimeout(long idleTimeout)
public void cancel()
{
if (LOG.isDebugEnabled()) LOG.debug("setIdleTimeout called: old="+getIdleTimeout()+" new="+idleTimeout);
super.setIdleTimeout(idleTimeout);
_timer.cancel();
if (LOG.isDebugEnabled()) LOG.debug("Cancelled timer for session {}",getId());
}
public void destroy ()
{
_timer.destroy();
if (LOG.isDebugEnabled()) LOG.debug("Destroyed timer for session {}",getId());
}
}
@ -171,6 +179,7 @@ public class Session implements SessionHandler.SessionIf
_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();
}
@ -186,6 +195,7 @@ public class Session implements SessionHandler.SessionIf
{
_handler = handler;
_sessionData = data;
_sessionInactivityTimer = new SessionInactivityTimer();
}
@ -231,17 +241,21 @@ public class Session implements SessionHandler.SessionIf
return false;
_newSession=false;
long lastAccessed = _sessionData.getAccessed();
if (_sessionInactivityTimer != null)
_sessionInactivityTimer.notIdle();
_sessionData.setAccessed(time);
_sessionData.setLastAccessed(lastAccessed);
_sessionData.calcAndSetExpiry(time);
_sessionData.calcAndSetExpiry(time);
if (isExpiredAt(time))
{
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;
}
}
@ -252,6 +266,12 @@ public class Session implements SessionHandler.SessionIf
try (Lock lock = _lock.lock())
{
_requests--;
if (LOG.isDebugEnabled()) LOG.debug("Session {} complete, active requests={}",getId(),_requests);
//start the inactivity timer
if (_requests == 0)
_sessionInactivityTimer.schedule();
}
}
@ -499,8 +519,6 @@ public class Session implements SessionHandler.SessionIf
{
try (Lock lock = _lock.lock())
{
if (LOG.isDebugEnabled())LOG.debug("updateInactivityTimer");
long maxInactive = _sessionData.getMaxInactiveMs();
int evictionPolicy = getSessionHandler().getSessionCache().getEvictionPolicy();
@ -510,64 +528,43 @@ public class Session implements SessionHandler.SessionIf
if (evictionPolicy < SessionCache.EVICT_ON_INACTIVITY)
{
//we do not want to evict inactive sessions
setInactivityTimer(-1L);
if (LOG.isDebugEnabled()) LOG.debug("Session is immortal && no inactivity eviction: timer cancelled");
_sessionInactivityTimer.setTimeout(-1);
if (LOG.isDebugEnabled()) LOG.debug("Session {} is immortal && no inactivity eviction", getId());
}
else
{
//sessions are immortal but we want to evict after inactivity
setInactivityTimer(TimeUnit.SECONDS.toMillis(evictionPolicy));
if (LOG.isDebugEnabled()) LOG.debug("Session is immortal; evict after {} sec inactivity", evictionPolicy);
_sessionInactivityTimer.setTimeout(TimeUnit.SECONDS.toMillis(evictionPolicy));
if (LOG.isDebugEnabled()) LOG.debug("Session {} is immortal; evict after {} sec inactivity", getId(), evictionPolicy);
}
}
else
{
//sessions are not immortal
if (evictionPolicy < SessionCache.EVICT_ON_INACTIVITY)
if (evictionPolicy == SessionCache.NEVER_EVICT)
{
//don't want to evict inactive sessions, set the timer for the session's maxInactive setting
setInactivityTimer(_sessionData.getMaxInactiveMs());
if (LOG.isDebugEnabled()) LOG.debug("No inactive session eviction");
//timeout is just the maxInactive setting
_sessionInactivityTimer.setTimeout(_sessionData.getMaxInactiveMs());
if (LOG.isDebugEnabled()) LOG.debug("Session {} no eviction", getId());
}
else if (evictionPolicy == SessionCache.EVICT_ON_SESSION_EXIT)
{
//session will not remain in the cache, so no timeout
_sessionInactivityTimer.setTimeout(-1);
if (LOG.isDebugEnabled()) LOG.debug("Session {} evict on exit", getId());
}
else
{
//set the time to the lesser of the session's maxInactive and eviction timeout
setInactivityTimer(Math.min(maxInactive, TimeUnit.SECONDS.toMillis(evictionPolicy)));
if (LOG.isDebugEnabled()) LOG.debug("Inactivity timer set to lesser of maxInactive={} and inactivityEvict={}", maxInactive, evictionPolicy);
//want to evict on idle: timer is lesser of the session's maxInactive and eviction timeout
_sessionInactivityTimer.setTimeout(Math.min(maxInactive, TimeUnit.SECONDS.toMillis(evictionPolicy)));
if (LOG.isDebugEnabled()) LOG.debug("Session {} timer set to lesser of maxInactive={} and inactivityEvict={}", getId(), maxInactive, evictionPolicy);
}
}
}
}
/**
* Set the inactivity timer
*
* @param ms value in millisec, -1 disables it
*/
private void setInactivityTimer (long ms)
{
if (_sessionInactivityTimer == null)
_sessionInactivityTimer = new SessionInactivityTimeout();
_sessionInactivityTimer.setIdleTimeout(ms);
}
/**
*
*/
public void stopInactivityTimer ()
{
try (Lock lock = _lock.lock())
{
if (_sessionInactivityTimer != null)
{
_sessionInactivityTimer.setIdleTimeout(-1);
_sessionInactivityTimer = null;
if (LOG.isDebugEnabled()) LOG.debug("Session timer stopped");
}
}
}
/**
* @see javax.servlet.http.HttpSession#getMaxInactiveInterval()
*/
@ -738,7 +735,7 @@ public class Session implements SessionHandler.SessionIf
Iterator<String> itor = _sessionData.getKeys().iterator();
if (!itor.hasNext())
return new String[0];
ArrayList<String> names = new ArrayList<String>();
ArrayList<String> names = new ArrayList<>();
while (itor.hasNext())
names.add(itor.next());
return names.toArray(new String[names.size()]);
@ -1019,6 +1016,11 @@ public class Session implements SessionHandler.SessionIf
public void setResident (boolean resident)
{
_resident = resident;
if (_resident)
updateInactivityTimer();
else
_sessionInactivityTimer.destroy();
}
/* ------------------------------------------------------------- */

View File

@ -131,7 +131,7 @@ public class SessionHandler extends ScopedHandler
public Set<SessionTrackingMode> __defaultSessionTrackingModes =
Collections.unmodifiableSet(
new HashSet<SessionTrackingMode>(
new HashSet<>(
Arrays.asList(new SessionTrackingMode[]{SessionTrackingMode.COOKIE,SessionTrackingMode.URL})));
@ -1103,7 +1103,7 @@ public class SessionHandler extends ScopedHandler
/* ------------------------------------------------------------ */
public void setSessionTrackingModes(Set<SessionTrackingMode> sessionTrackingModes)
{
_sessionTrackingModes=new HashSet<SessionTrackingMode>(sessionTrackingModes);
_sessionTrackingModes=new HashSet<>(sessionTrackingModes);
_usingCookies=_sessionTrackingModes.contains(SessionTrackingMode.COOKIE);
_usingURLs=_sessionTrackingModes.contains(SessionTrackingMode.URL);
}
@ -1262,7 +1262,7 @@ public class SessionHandler extends ScopedHandler
//arrive during this processing will be dealt with on
//subsequent call to scavenge
String[] ss = _candidateSessionIdsForExpiry.toArray(new String[0]);
Set<String> candidates = new HashSet<String>(Arrays.asList(ss));
Set<String> candidates = new HashSet<>(Arrays.asList(ss));
_candidateSessionIdsForExpiry.removeAll(candidates);
if (LOG.isDebugEnabled())
LOG.debug("{} scavenging session ids {}", this, candidates);

View File

@ -136,7 +136,7 @@ public class SessionCookieTest
long now = TimeUnit.NANOSECONDS.toMillis(System.nanoTime());
Session session = new Session(null, new SessionData("123", "_foo", "0.0.0.0", now, now, now, 30));
Session session = new Session(mgr, new SessionData("123", "_foo", "0.0.0.0", now, now, now, 30));
SessionCookieConfig sessionCookieConfig = mgr.getSessionCookieConfig();
sessionCookieConfig.setSecure(true);

View File

@ -110,7 +110,6 @@ public class IdleSessionTest
//check session reactivated
assertTrue(contextHandler.getSessionHandler().getSessionCache().contains(id));
//wait again for the session to be passivated
pause(evictionSec*2);
@ -165,6 +164,95 @@ public class IdleSessionTest
}
@Test
public void testNullSessionCache () throws Exception
{
//test the NullSessionCache which does not support idle timeout
String contextPath = "";
String servletMapping = "/server";
int inactivePeriod = 20;
int scavengePeriod = 3;
NullSessionCacheFactory cacheFactory = new NullSessionCacheFactory();
SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
_server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
ServletHolder holder = new ServletHolder(_servlet);
ServletContextHandler contextHandler = _server1.addContext(contextPath);
contextHandler.addServlet(holder, servletMapping);
_server1.start();
int port1 = _server1.getPort();
try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
{
HttpClient client = new HttpClient();
client.start();
String url = "http://localhost:" + port1 + contextPath + servletMapping;
//make a request to set up a session on the server
ContentResponse response = client.GET(url + "?action=init");
assertEquals(HttpServletResponse.SC_OK,response.getStatus());
String sessionCookie = response.getHeaders().get("Set-Cookie");
assertTrue(sessionCookie != null);
//the session should never be cached
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
Request request = client.newRequest(url + "?action=test");
ContentResponse response2 = request.send();
assertEquals(HttpServletResponse.SC_OK,response2.getStatus());
//check session still not in the cache
assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
//While passivated, take some action to ensure that a reactivate won't work, like
//deleting the sessions in the store
((TestSessionDataStore)contextHandler.getSessionHandler().getSessionCache().getSessionDataStore())._map.clear();
//make a request
request = client.newRequest(url + "?action=testfail");
response2 = request.send();
assertEquals(HttpServletResponse.SC_OK,response2.getStatus());
//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
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);
//stop the scavenger
if (_server1.getHouseKeeper() != null)
_server1.getHouseKeeper().stop();
//check that the session is passivated
assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
//wait until the session should be expired
pause (inactivePeriod + (3*scavengePeriod));
//make another request to reactivate the session
request = client.newRequest(url + "?action=testfail");
response2 = request.send();
assertEquals(HttpServletResponse.SC_OK,response2.getStatus());
assertFalse(contextHandler.getSessionHandler().getSessionCache().contains(id));
assertFalse(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(id));
}
finally
{
_server1.stop();
}
}

View File

@ -0,0 +1,76 @@
//
// ========================================================================
// Copyright (c) 1995-2018 Mort Bay Consulting Pty. Ltd.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//
package org.eclipse.jetty.server.session;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.util.concurrent.TimeUnit;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.junit.Test;
/**
* NullSessionCacheTest
*
*
*/
public class NullSessionCacheTest
{
@Test
public void testEvictOnExit() throws Exception
{
Server server = new Server();
ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS);
context.setContextPath("/test");
context.setServer(server);
NullSessionCacheFactory cacheFactory = new NullSessionCacheFactory();
NullSessionCache cache = (NullSessionCache)cacheFactory.getSessionCache(context.getSessionHandler());
TestSessionDataStore store = new TestSessionDataStore();
cache.setSessionDataStore(store);
context.getSessionHandler().setSessionCache(cache);
context.start();
//make a session
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));
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"));
assertTrue(store.exists("1234"));
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
assertFalse(session.isResident());
}
}

View File

@ -61,7 +61,7 @@ public class SameNodeLoadTest
String contextPath = "";
String servletMapping = "/server";
TestServer server1 = new TestServer(0, -1, 4, cacheFactory, storeFactory);
TestServer server1 = new TestServer(0, 60, 5, cacheFactory, storeFactory);
server1.addContext( contextPath ).addServlet( TestServlet.class, servletMapping );