From 8920fcc1280e9c1ad0c3b5a07770efa647fc7913 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Thu, 22 Nov 2012 15:49:11 +1100 Subject: [PATCH] 394829 Session can not be restored after SessionManager.setIdleSavePeriod has saved the session --- .../server/session/HashSessionManager.java | 41 ++-- .../jetty/server/session/HashedSession.java | 12 +- .../jetty/server/session/HashTestServer.java | 2 +- .../jetty/server/session/IdleSessionTest.java | 217 ++++++++++++++++++ 4 files changed, 252 insertions(+), 20 deletions(-) create mode 100644 tests/test-sessions/test-hash-sessions/src/test/java/org/eclipse/jetty/server/session/IdleSessionTest.java diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionManager.java index 0f789328fd5..738302f79b4 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionManager.java @@ -40,7 +40,10 @@ import org.eclipse.jetty.util.log.Logger; /* ------------------------------------------------------------ */ -/** An in-memory implementation of SessionManager. +/** + * HashSessionManager + * + * An in-memory implementation of SessionManager. *

* This manager supports saving sessions to disk, either periodically or at shutdown. * Sessions can also have their content idle saved to disk to reduce the memory overheads of large idle sessions. @@ -77,7 +80,7 @@ public class HashSessionManager extends AbstractSessionManager } /* ------------------------------------------------------------ */ - /* (non-Javadoc) + /** * @see org.eclipse.jetty.servlet.AbstractSessionManager#doStart() */ @Override @@ -110,7 +113,7 @@ public class HashSessionManager extends AbstractSessionManager } /* ------------------------------------------------------------ */ - /* (non-Javadoc) + /** * @see org.eclipse.jetty.servlet.AbstractSessionManager#doStop() */ @Override @@ -252,7 +255,7 @@ public class HashSessionManager extends AbstractSessionManager * @param seconds the period in seconds at which a check is made for sessions to be invalidated. */ public void setScavengePeriod(int seconds) - { + { if (seconds==0) seconds=60; @@ -264,6 +267,7 @@ public class HashSessionManager extends AbstractSessionManager period=1000; _scavengePeriodMs=period; + if (_timer!=null && (period!=old_period || _task==null)) { synchronized (this) @@ -303,25 +307,36 @@ public class HashSessionManager extends AbstractSessionManager // For each session long now=System.currentTimeMillis(); + for (Iterator i=_sessions.values().iterator(); i.hasNext();) { HashedSession session=i.next(); - long idleTime=session.getMaxInactiveInterval()*1000L; + long idleTime=session.getMaxInactiveInterval()*1000L; if (idleTime>0&&session.getAccessed()+idleTime0&&session.getAccessed()+_idleSavePeriodMs 0 && session.getAccessed()+_idleSavePeriodMs < now) { - session.idle(); + try + { + session.idle(); + } + catch (Exception e) + { + __log.warn("Problem idling session "+ session.getId(), e); + } } } - } - catch (Throwable t) - { - __log.warn("Problem scavenging sessions", t); - } + } finally { thread.setContextClassLoader(old_loader); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashedSession.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashedSession.java index 33119f4f977..265680b98a9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashedSession.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashedSession.java @@ -100,6 +100,7 @@ public class HashedSession extends AbstractSession /* ------------------------------------------------------------ */ synchronized void save(boolean reactivate) + throws Exception { // Only idle the session if not already idled and no previous save/idle has failed if (!isIdled() && !_saveFailed) @@ -128,16 +129,13 @@ public class HashedSession extends AbstractSession catch (Exception e) { saveFailed(); // We won't try again for this session - - LOG.warn("Problem saving session " + super.getId(), e); - if (fos != null) { // Must not leave the file open if the saving failed IO.close(fos); // No point keeping the file if we didn't save the whole session file.delete(); - _idled=false; // assume problem was before _values.clear(); + throw e; } } } @@ -181,7 +179,7 @@ public class HashedSession extends AbstractSession access(System.currentTimeMillis()); if (LOG.isDebugEnabled()) - LOG.debug("Deidling " + super.getId()); + LOG.debug("De-idling " + super.getId()); FileInputStream fis = null; @@ -203,7 +201,7 @@ public class HashedSession extends AbstractSession } catch (Exception e) { - LOG.warn("Problem deidling session " + super.getId(), e); + LOG.warn("Problem de-idling session " + super.getId(), e); IO.close(fis); invalidate(); } @@ -219,8 +217,10 @@ public class HashedSession extends AbstractSession * it to an idled state. */ public synchronized void idle() + throws Exception { save(false); + _idled = true; } /* ------------------------------------------------------------ */ diff --git a/tests/test-sessions/test-hash-sessions/src/test/java/org/eclipse/jetty/server/session/HashTestServer.java b/tests/test-sessions/test-hash-sessions/src/test/java/org/eclipse/jetty/server/session/HashTestServer.java index 924a51d6184..25c3ba59ff7 100644 --- a/tests/test-sessions/test-hash-sessions/src/test/java/org/eclipse/jetty/server/session/HashTestServer.java +++ b/tests/test-sessions/test-hash-sessions/src/test/java/org/eclipse/jetty/server/session/HashTestServer.java @@ -48,7 +48,7 @@ public class HashTestServer extends AbstractTestServer public SessionManager newSessionManager() { HashSessionManager manager = new HashSessionManager(); - manager.setScavengePeriod((int)TimeUnit.SECONDS.toMillis(_scavengePeriod)); + manager.setScavengePeriod(_scavengePeriod); return manager; } diff --git a/tests/test-sessions/test-hash-sessions/src/test/java/org/eclipse/jetty/server/session/IdleSessionTest.java b/tests/test-sessions/test-hash-sessions/src/test/java/org/eclipse/jetty/server/session/IdleSessionTest.java new file mode 100644 index 00000000000..18dbb430e34 --- /dev/null +++ b/tests/test-sessions/test-hash-sessions/src/test/java/org/eclipse/jetty/server/session/IdleSessionTest.java @@ -0,0 +1,217 @@ +// +// ======================================================================== +// Copyright (c) 1995-2012 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.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotNull; + + +import java.io.File; +import java.io.IOException; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; + +import org.eclipse.jetty.client.ContentExchange; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.http.HttpMethods; +import org.eclipse.jetty.server.SessionManager; +import org.eclipse.jetty.server.session.AbstractSessionExpiryTest.TestServlet; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.util.IO; +import org.junit.Test; + + +/** + * IdleSessionTest + * + * Checks that a session can be idled and de-idled on the next request if it hasn't expired. + * + */ +public class IdleSessionTest +{ + public class IdleHashTestServer extends HashTestServer + { + private int _idlePeriod; + private File _storeDir; + + /** + * @param port + * @param maxInactivePeriod + * @param scavengePeriod + * @param idlePeriod + */ + public IdleHashTestServer(int port, int maxInactivePeriod, int scavengePeriod, int idlePeriod, File storeDir) + { + super(port, maxInactivePeriod, scavengePeriod); + _idlePeriod = idlePeriod; + _storeDir = storeDir; + } + + @Override + public SessionManager newSessionManager() + { + HashSessionManager manager = (HashSessionManager)super.newSessionManager(); + manager.setStoreDirectory(_storeDir); + manager.setIdleSavePeriod(_idlePeriod); + return manager; + } + + + + } + + public HashTestServer createServer(int port, int max, int scavenge, int idle, File storeDir) + { + HashTestServer server = new IdleHashTestServer(port, max, scavenge, idle, storeDir); + return server; + } + + + + public void pause (int sec) + { + try + { + Thread.sleep(sec * 1000L); + } + catch (InterruptedException e) + { + e.printStackTrace(); + } + } + + @Test + public void testSessionIdle() throws Exception + { + String contextPath = ""; + String servletMapping = "/server"; + int inactivePeriod = 200; + int scavengePeriod = 3; + int idlePeriod = 5; + + File storeDir = new File (System.getProperty("java.io.tmpdir"), "idle-test"); + storeDir.deleteOnExit(); + + HashTestServer server1 = createServer(0, inactivePeriod, scavengePeriod, idlePeriod, storeDir); + TestServlet servlet = new TestServlet(); + ServletHolder holder = new ServletHolder(servlet); + ServletContextHandler contextHandler = server1.addContext(contextPath); + contextHandler.addServlet(holder, servletMapping); + server1.start(); + int port1 = server1.getPort(); + + try + { + HttpClient client = new HttpClient(); + client.setConnectorType(HttpClient.CONNECTOR_SOCKET); + client.start(); + String url = "http://localhost:" + port1 + contextPath + servletMapping; + + //make a request to set up a session on the server + ContentExchange exchange1 = new ContentExchange(true); + exchange1.setMethod(HttpMethods.GET); + exchange1.setURL(url + "?action=init"); + client.send(exchange1); + exchange1.waitForDone(); + assertEquals(HttpServletResponse.SC_OK,exchange1.getResponseStatus()); + String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie"); + assertTrue(sessionCookie != null); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //and wait until the session should be idled out + pause(scavengePeriod * 2); + + //check that the file exists + checkSessionIdled(storeDir); + + //make another request to de-idle the session + ContentExchange exchange2 = new ContentExchange(true); + exchange2.setMethod(HttpMethods.GET); + exchange2.setURL(url + "?action=test"); + exchange2.getRequestFields().add("Cookie", sessionCookie); + client.send(exchange2); + exchange2.waitForDone(); + assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus()); + + //check session de-idled + checkSessionDeIdled(storeDir); + + } + finally + { + server1.stop(); + IO.delete(storeDir); + } + } + + + public void checkSessionIdled (File sessionDir) + { + assertNotNull(sessionDir); + assertTrue(sessionDir.exists()); + String[] files = sessionDir.list(); + assertNotNull(files); + assertEquals(1, files.length); + } + + + public void checkSessionDeIdled (File sessionDir) + { + assertNotNull(sessionDir); + assertTrue(sessionDir.exists()); + String[] files = sessionDir.list(); + assertNotNull(files); + assertEquals(0, files.length); + } + + + public static class TestServlet extends HttpServlet + { + public String originalId = null; + public String testId = null; + public String checkId = null; + + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse httpServletResponse) throws ServletException, IOException + { + String action = request.getParameter("action"); + if ("init".equals(action)) + { + HttpSession session = request.getSession(true); + session.setAttribute("test", "test"); + originalId = session.getId(); + assertTrue(!((HashedSession)session).isIdled()); + } + else if ("test".equals(action)) + { + HttpSession session = request.getSession(false); + assertTrue(session != null); + assertTrue(originalId.equals(session.getId())); + assertEquals("test", session.getAttribute("test")); + assertTrue(!((HashedSession)session).isIdled()); + } + } + } +}