diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java index e15a47d6a2f..a72c0702b09 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java @@ -821,7 +821,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager statement = connection.prepareStatement(_deleteOldExpiredSessions); statement.setLong(1, upperBound); int rows = statement.executeUpdate(); - if (LOG.isDebugEnabled()) LOG.debug("Deleted "+rows+" rows"); + if (LOG.isDebugEnabled()) LOG.debug("Deleted "+rows+" rows of old sessions expired before "+upperBound); } } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java index 4bb8815b02d..ffdfa41e966 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java @@ -279,6 +279,8 @@ public class JDBCSessionManager extends AbstractSessionManager return false; } } + + /** * Exit from session @@ -330,7 +332,7 @@ public class JDBCSessionManager extends AbstractSessionManager return "Session rowId="+_rowId+",id="+getId()+",lastNode="+_lastNode+ ",created="+getCreationTime()+",accessed="+getAccessed()+ ",lastAccessed="+getLastAccessedTime()+",cookieSet="+_cookieSet+ - "lastSaved="+_lastSaved; + ",lastSaved="+_lastSaved+",expiry="+_expiryTime; } } @@ -440,8 +442,6 @@ public class JDBCSessionManager extends AbstractSessionManager synchronized (this) { - try - { //check if we need to reload the session - //as an optimization, don't reload on every access //to reduce the load on the database. This introduces a window of @@ -469,22 +469,31 @@ public class JDBCSessionManager extends AbstractSessionManager " difference="+(now - memSession._lastSaved)); } - if (memSession==null) + try { - LOG.debug("getSession("+idInCluster+"): no session in session map. Reloading session data from db."); - session = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); + if (memSession==null) + { + LOG.debug("getSession("+idInCluster+"): no session in session map. Reloading session data from db."); + session = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); + } + else if ((now - memSession._lastSaved) >= (_saveIntervalSec * 1000L)) + { + LOG.debug("getSession("+idInCluster+"): stale session. Reloading session data from db."); + session = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); + } + else + { + LOG.debug("getSession("+idInCluster+"): session in session map"); + session = memSession; + } } - else if ((now - memSession._lastSaved) >= (_saveIntervalSec * 1000L)) + catch (Exception e) { - LOG.debug("getSession("+idInCluster+"): stale session. Reloading session data from db."); - session = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); - } - else - { - LOG.debug("getSession("+idInCluster+"): session in session map"); - session = memSession; + LOG.warn("Unable to load session "+idInCluster, e); + return null; } + //If we have a session if (session != null) { @@ -494,13 +503,23 @@ public class JDBCSessionManager extends AbstractSessionManager //if session doesn't expire, or has not already expired, update it and put it in this nodes' memory if (session._expiryTime <= 0 || session._expiryTime > now) { - if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): lastNode="+session.getLastNode()+" thisNode="+getSessionIdManager().getWorkerName()); + if (LOG.isDebugEnabled()) + LOG.debug("getSession("+idInCluster+"): lastNode="+session.getLastNode()+" thisNode="+getSessionIdManager().getWorkerName()); + session.setLastNode(getSessionIdManager().getWorkerName()); _sessions.put(idInCluster, session); - session.didActivate(); - //TODO is this the best way to do this? Or do this on the way out using - //the _dirty flag? - updateSessionNode(session); + + //update in db: if unable to update, session will be scavenged later + try + { + updateSessionNode(session); + session.didActivate(); + } + catch (Exception e) + { + LOG.warn("Unable to update freshly loaded session "+idInCluster, e); + return null; + } } else { @@ -519,12 +538,6 @@ public class JDBCSessionManager extends AbstractSessionManager } return session; - } - catch (Exception e) - { - LOG.warn("Unable to get session", e); - return null; - } } } @@ -840,7 +853,12 @@ public class JDBCSessionManager extends AbstractSessionManager _context.getContextHandler().handle(load); if (_exception.get()!=null) + { + //if the session could not be restored, take its id out of the pool of currently-in-use + //session ids + _jdbcSessionIdMgr.removeSession(id); throw _exception.get(); + } return _reference.get(); } diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JdbcTestServer.java b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JdbcTestServer.java index d65213c9092..dbdcd47e511 100644 --- a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JdbcTestServer.java +++ b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JdbcTestServer.java @@ -53,11 +53,7 @@ public class JdbcTestServer extends AbstractTestServer super(port, maxInactivePeriod, scavengePeriod, DEFAULT_CONNECTION_URL); } - public JdbcTestServer (int port, boolean optimize) - { - super(port); - } - + /** * @see org.eclipse.jetty.server.session.AbstractTestServer#newSessionHandler(org.eclipse.jetty.server.SessionManager) */ diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ReloadedSessionMissingClassTest.java b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ReloadedSessionMissingClassTest.java new file mode 100644 index 00000000000..54094cd206f --- /dev/null +++ b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ReloadedSessionMissingClassTest.java @@ -0,0 +1,153 @@ +// +// ======================================================================== +// Copyright (c) 1995-2013 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.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.FileWriter; +import java.net.URL; +import java.net.URLClassLoader; + +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.client.ContentExchange; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.http.HttpMethods; +import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.StdErrLog; +import org.eclipse.jetty.webapp.WebAppContext; +import org.junit.Test; + +/** + * ReloadedSessionMissingClassTest + * + * + * + */ +public class ReloadedSessionMissingClassTest +{ + + @Test + public void testSessionReloadWithMissingClass() throws Exception + { + ((StdErrLog)Log.getLogger(org.eclipse.jetty.server.session.JDBCSessionManager.class)).setHideStacks(true); + String contextPath = "/foo"; + File srcDir = new File(System.getProperty("basedir"), "src"); + File targetDir = new File(System.getProperty("basedir"), "target"); + File testDir = new File (srcDir, "test"); + File resourcesDir = new File (testDir, "resources"); + + File unpackedWarDir = new File (targetDir, "foo"); + if (unpackedWarDir.exists()) + IO.delete(unpackedWarDir); + unpackedWarDir.mkdir(); + + File webInfDir = new File (unpackedWarDir, "WEB-INF"); + webInfDir.mkdir(); + + File webXml = new File(webInfDir, "web.xml"); + String xml = + "\n" + + "\n" + + "\n" + + "\n"+ + " 1\n" + + "\n"+ + ""; + FileWriter w = new FileWriter(webXml); + w.write(xml); + w.close(); + + File foobarJar = new File (resourcesDir, "foobar.jar"); + File foobarNOfooJar = new File (resourcesDir, "foobarNOfoo.jar"); + + URL[] foobarUrls = new URL[]{foobarJar.toURI().toURL()}; + URL[] barUrls = new URL[]{foobarNOfooJar.toURI().toURL()}; + + URLClassLoader loaderWithFoo = new URLClassLoader(foobarUrls, Thread.currentThread().getContextClassLoader()); + URLClassLoader loaderWithoutFoo = new URLClassLoader(barUrls, Thread.currentThread().getContextClassLoader()); + + + AbstractTestServer server1 = new JdbcTestServer(0); + WebAppContext webApp = server1.addWebAppContext(unpackedWarDir.getCanonicalPath(), contextPath); + webApp.setClassLoader(loaderWithFoo); + webApp.addServlet("Bar", "/bar"); + server1.start(); + int port1 = server1.getPort(); + try + { + HttpClient client = new HttpClient(); + client.setConnectorType(HttpClient.CONNECTOR_SOCKET); + client.start(); + try + { + // Perform one request to server1 to create a session + ContentExchange exchange1 = new ContentExchange(true); + exchange1.setMethod(HttpMethods.GET); + exchange1.setURL("http://localhost:" + port1 + contextPath +"/bar?action=set"); + client.send(exchange1); + exchange1.waitForDone(); + assertEquals( HttpServletResponse.SC_OK, exchange1.getResponseStatus()); + String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie"); + assertTrue(sessionCookie != null); + String sessionId = (String)webApp.getServletContext().getAttribute("foo"); + assertNotNull(sessionId); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //Stop the webapp + webApp.stop(); + + webApp.setClassLoader(loaderWithoutFoo); + webApp.addServlet("Bar", "/bar"); + + //restart webapp + webApp.start(); + + ContentExchange exchange2 = new ContentExchange(true); + exchange2.setMethod(HttpMethods.GET); + exchange2.setURL("http://localhost:" + port1 + contextPath + "/bar?action=get"); + exchange2.getRequestFields().add("Cookie", sessionCookie); + client.send(exchange2); + exchange2.waitForDone(); + assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus()); + String afterStopSessionId = (String)webApp.getServletContext().getAttribute("foo.session"); + + assertNotNull(afterStopSessionId); + assertTrue(!afterStopSessionId.equals(sessionId)); + + } + finally + { + client.stop(); + } + } + finally + { + server1.stop(); + } + } +} diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobar.jar b/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobar.jar new file mode 100644 index 00000000000..29b46ddee9f Binary files /dev/null and b/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobar.jar differ diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobarNOfoo.jar b/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobarNOfoo.jar new file mode 100644 index 00000000000..fcb3ddf78c9 Binary files /dev/null and b/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobarNOfoo.jar differ