diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java index b3022551acd..9c14ac573a8 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.server.session; import java.io.IOException; import java.util.Collections; +import java.util.HashSet; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -651,7 +652,28 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements if (LOG.isDebugEnabled()) LOG.debug("SessionDataStore checking expiration on {}", candidates); - return _sessionDataStore.getExpired(candidates); + Set allCandidates = _sessionDataStore.getExpired(candidates); + Set sessionsInUse = new HashSet<>(); + if (allCandidates != null) + { + for (String c:allCandidates) + { + Session s = doGet(c); + if (s != null && s.getRequests() > 0) //if the session is in my cache, check its not in use first + sessionsInUse.add(c); + } + try + { + allCandidates.removeAll(sessionsInUse); + } + catch (UnsupportedOperationException e) + { + Set tmp = new HashSet<>(allCandidates); + tmp.removeAll(sessionsInUse); + allCandidates = tmp; + } + } + return allCandidates; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java index 50d254d143b..5bbb9f1c3f3 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java @@ -149,7 +149,7 @@ public class Session implements SessionHandler.SessionIf @Override public void setIdleTimeout(long idleTimeout) { - if (LOG.isDebugEnabled()) LOG.debug("setIdleTimeout called: "+idleTimeout); + if (LOG.isDebugEnabled()) LOG.debug("setIdleTimeout called: old="+getIdleTimeout()+" new="+idleTimeout); super.setIdleTimeout(idleTimeout); } @@ -231,6 +231,8 @@ 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); @@ -889,6 +891,16 @@ public class Session implements SessionHandler.SessionIf { return _lock.lock(); } + + + /* ------------------------------------------------------------- */ + /** Grab the lock on the session if it isn't locked already + * @return the lock + */ + public Lock lockIfNotHeld () + { + return _lock.lockIfNotHeld(); + } /* ------------------------------------------------------------- */ /** Call HttpSessionAttributeListeners as part of invalidating diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java index 49e4129cd1c..ddaf1f47ed7 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java @@ -1292,15 +1292,18 @@ public class SessionHandler extends ScopedHandler { if (session == null) return; - + //check if the session is: //1. valid //2. expired //3. idle boolean expired = false; - try (Lock lock = session.lock()) + try (Lock lock = session.lockIfNotHeld()) { + if (session.getRequests() > 0) + return; //session can't expire or be idle if there is a request in it + if (LOG.isDebugEnabled()) LOG.debug("Inspecting session {}, valid={}", session.getId(), session.isValid()); diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ModifyMaxInactiveIntervalTest.java b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ModifyMaxInactiveIntervalTest.java index 045fc3a320d..7cdca47c968 100644 --- a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ModifyMaxInactiveIntervalTest.java +++ b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ModifyMaxInactiveIntervalTest.java @@ -46,15 +46,339 @@ import org.junit.Test; public class ModifyMaxInactiveIntervalTest { - public static int __inactive = 4; + public static int newMaxInactive = 20; public static int __scavenge = 1; + + @Test + public void testReduceMaxInactiveInterval() throws Exception + { + int oldMaxInactive = 3; + int newMaxInactive = 1; + int sleep = (int)(oldMaxInactive * 0.8); + + + AbstractTestServer server = new JdbcTestServer(0, oldMaxInactive, 1, SessionCache.NEVER_EVICT); + ServletContextHandler ctxA = server.addContext("/mod"); + ctxA.addServlet(TestModServlet.class, "/test"); + + server.start(); + int port=server.getPort(); + try + { + HttpClient client = new HttpClient(); + client.start(); + try + { + // Perform a request to create a session + 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); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //do another request to reduce the maxinactive interval + Request request = client.newRequest("http://localhost:" + port + "/mod/test?action=change&val="+newMaxInactive+"&wait="+sleep); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //do another request using the cookie to ensure the session is still there + request= client.newRequest("http://localhost:" + port + "/mod/test?action=test&val="+newMaxInactive); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + } + finally + { + client.stop(); + } + } + finally + { + server.stop(); + } + } + + + @Test + public void testIncreaseMaxInactiveInterval() throws Exception + { + + int oldMaxInactive = 3; + int newMaxInactive = 5; + int sleep = (int)(oldMaxInactive * 0.8); + + AbstractTestServer server = new JdbcTestServer(0, oldMaxInactive, 1, SessionCache.NEVER_EVICT); + ServletContextHandler ctxA = server.addContext("/mod"); + ctxA.addServlet(TestModServlet.class, "/test"); + + server.start(); + int port=server.getPort(); + try + { + HttpClient client = new HttpClient(); + client.start(); + try + { + // Perform a request to create a session + 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); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //do another request to increase the maxinactive interval, first waiting until the old expiration should have passed + Request request = client.newRequest("http://localhost:" + port + "/mod/test?action=change&val="+newMaxInactive+"&wait="+sleep); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //do another request using the cookie to ensure the session is still there + request= client.newRequest("http://localhost:" + port + "/mod/test?action=test&val="+newMaxInactive); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + } + finally + { + client.stop(); + } + } + finally + { + server.stop(); + } + } + + + @Test + public void testSetMaxInactiveIntervalWithImmortalSessionAndEviction() throws Exception + { + int oldMaxInactive = -1; + int newMaxInactive = 120; //2min + int evict = 2; + int sleep = evict; + + AbstractTestServer server = new JdbcTestServer(0, oldMaxInactive, 1, evict); + + + ServletContextHandler ctxA = server.addContext("/mod"); + ctxA.addServlet(TestModServlet.class, "/test"); + + server.start(); + int port=server.getPort(); + try + { + HttpClient client = new HttpClient(); + client.start(); + try + { + // Perform a request to create a session + 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); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //do another request to reduce the maxinactive interval + Request request = client.newRequest("http://localhost:" + port + "/mod/test?action=change&val="+newMaxInactive+"&wait="+sleep); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //do another request using the cookie to ensure the session is still there + request= client.newRequest("http://localhost:" + port + "/mod/test?action=test&val="+newMaxInactive); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + } + finally + { + client.stop(); + } + } + finally + { + server.stop(); + } + } + + + @Test + public void testSetMaxInactiveIntervalWithNonImmortalSessionAndEviction() throws Exception + { + int oldMaxInactive = 10; + int newMaxInactive = 2; + int evict = 4; + int sleep = evict; + + AbstractTestServer server = new JdbcTestServer(0, oldMaxInactive, 1, evict); + ServletContextHandler ctxA = server.addContext("/mod"); + ctxA.addServlet(TestModServlet.class, "/test"); + + server.start(); + int port=server.getPort(); + try + { + HttpClient client = new HttpClient(); + client.start(); + try + { + // Perform a request to create a session + 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); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //do another request to reduce the maxinactive interval + Request request = client.newRequest("http://localhost:" + port + "/mod/test?action=change&val="+newMaxInactive+"&wait="+sleep); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //do another request using the cookie to ensure the session is still there + request= client.newRequest("http://localhost:" + port + "/mod/test?action=test&val="+newMaxInactive); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + } + finally + { + client.stop(); + } + } + finally + { + server.stop(); + } + } + + @Test + public void testChangeMaxInactiveIntervalForImmortalSessionNoEviction() throws Exception + { + int oldMaxInactive = -1; + int newMaxInactive = 120; + + AbstractTestServer server = new JdbcTestServer(0, oldMaxInactive, 1, SessionCache.NEVER_EVICT); + + + ServletContextHandler ctxA = server.addContext("/mod"); + ctxA.addServlet(TestModServlet.class, "/test"); + + server.start(); + int port=server.getPort(); + try + { + HttpClient client = new HttpClient(); + client.start(); + try + { + // Perform a request to create a session + 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); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //do another request to change the maxinactive interval + Request request = client.newRequest("http://localhost:" + port + "/mod/test?action=change&val="+newMaxInactive+"&wait="+2); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + //do another request using the cookie to ensure the session is still there + request= client.newRequest("http://localhost:" + port + "/mod/test?action=test&val="+newMaxInactive); + request.header("Cookie", sessionCookie); + response = request.send(); + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + } + finally + { + client.stop(); + } + } + finally + { + server.stop(); + } + } + + @Test + public void testNoExpireSessionInUse() throws Exception + { + int maxInactive = 3; + int sleep = maxInactive + (int)(maxInactive * 0.8); + + AbstractTestServer server = new JdbcTestServer(0, maxInactive, 1, SessionCache.NEVER_EVICT); + + + ServletContextHandler ctxA = server.addContext("/mod"); + ctxA.addServlet(TestModServlet.class, "/test"); + + server.start(); + int port=server.getPort(); + try + { + HttpClient client = new HttpClient(); + client.start(); + try + { + // Perform a request to create a session + + 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); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //do another request that will sleep long enough for the session expiry time to have passed + //before trying to access the session and ensure it is still there + Request request = client.newRequest("http://localhost:" + port + "/mod/test?action=sleep&val="+sleep); + request.header("Cookie", sessionCookie); + response = request.send(); + + assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + } + finally + { + client.stop(); + } + } + finally + { + server.stop(); + } + } @Test public void testSessionExpiryAfterModifiedMaxInactiveInterval() throws Exception { - AbstractTestServer server = new JdbcTestServer(0,__inactive,__scavenge, SessionCache.NEVER_EVICT); + int oldMaxInactive = 4; + int newMaxInactive = 20; + int sleep = oldMaxInactive+(int)(oldMaxInactive * 0.8); + + AbstractTestServer server = new JdbcTestServer(0, oldMaxInactive,__scavenge, SessionCache.NEVER_EVICT); ServletContextHandler ctxA = server.addContext("/mod"); ctxA.addServlet(TestModServlet.class, "/test"); @@ -85,13 +409,15 @@ public class ModifyMaxInactiveIntervalTest assertEquals(HttpServletResponse.SC_OK,response.getStatus()); //wait for longer than the old inactive interval - Thread.currentThread().sleep(10*1000L); + Thread.currentThread().sleep(sleep*1000L); //do another request using the cookie to ensure the session is still there - request= client.newRequest("http://localhost:" + port + "/mod/test?action=test"); + request= client.newRequest("http://localhost:" + port + "/mod/test?action=test&val="+newMaxInactive); request.header("Cookie", sessionCookie); response = request.send(); assertEquals(HttpServletResponse.SC_OK,response.getStatus()); + + } finally { @@ -126,16 +452,51 @@ public class ModifyMaxInactiveIntervalTest if ("change".equals(action)) { + //change the expiry time for the session, maybe sleeping before the change + String tmp = request.getParameter("val"); + int interval = -1; + interval = (tmp==null?-1:Integer.parseInt(tmp)); + + tmp = request.getParameter("wait"); + int wait = (tmp==null?0:Integer.parseInt(tmp)); + if (wait >0) + { + try { Thread.currentThread().sleep(wait*1000);}catch (Exception e) {throw new ServletException(e);} + } HttpSession session = request.getSession(false); if (session == null) throw new ServletException("Session is null for action=change"); - String tmp = request.getParameter("val"); - int interval = -1; - interval = (tmp==null?-1:Integer.parseInt(tmp)); - if (interval > 0) - session.setMaxInactiveInterval(interval); + session.setMaxInactiveInterval(interval); + + session = request.getSession(false); + if (session == null) + throw new ServletException ("Null session after maxInactiveInterval change"); + return; + } + + if ("sleep".equals(action)) + { + //sleep before trying to access the session + + HttpSession session = request.getSession(false); + if (session == null) + throw new ServletException("Session is null for action=sleep"); + + String tmp = request.getParameter("val"); + int interval = 0; + interval = (tmp==null?0:Integer.parseInt(tmp)); + + if (interval > 0) + { + try{Thread.currentThread().sleep(interval*1000);}catch (Exception e) {throw new ServletException(e);} + } + + session = request.getSession(false); + if (session == null) + throw new ServletException("Session null after sleep"); + return; } @@ -144,7 +505,11 @@ public class ModifyMaxInactiveIntervalTest HttpSession session = request.getSession(false); if (session == null) throw new ServletException("Session does not exist"); - assertEquals(ModifyMaxInactiveIntervalTest.newMaxInactive, session.getMaxInactiveInterval()); + String tmp = request.getParameter("val"); + int interval = 0; + interval = (tmp==null?0:Integer.parseInt(tmp)); + + assertEquals(interval, session.getMaxInactiveInterval()); return; } }