From: markt Date: Thu, 1 May 2008 21:11:26 +0000 (+0000) Subject: Fix bug 43343. Correctly handle the case where a request arrives for a session we... X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=0a264713cd31469a399984c3af2bd0b7dc107512;p=tomcat7.0 Fix bug 43343. Correctly handle the case where a request arrives for a session we are in the middle of persisting. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@652662 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/java/org/apache/catalina/session/PersistentManagerBase.java b/java/org/apache/catalina/session/PersistentManagerBase.java index 9fc44864a..ce8fc1164 100644 --- a/java/org/apache/catalina/session/PersistentManagerBase.java +++ b/java/org/apache/catalina/session/PersistentManagerBase.java @@ -590,6 +590,23 @@ public abstract class PersistentManagerBase public Session findSession(String id) throws IOException { Session session = super.findSession(id); + // OK, at this point, we're not sure if another thread is trying to + // remove the session or not so the only way around this is to lock it + // (or attempt to) and then try to get it by this session id again. If + // the other code ran swapOut, then we should get a null back during + // this run, and if not, we lock it out so we can access the session + // safely. + if(session != null) { + synchronized(session){ + session = super.findSession(session.getIdInternal()); + if(session != null){ + // To keep any external calling code from messing up the + // concurrency. + session.access(); + session.endAccess(); + } + } + } if (session != null) return (session); @@ -1024,24 +1041,24 @@ public abstract class PersistentManagerBase long timeNow = System.currentTimeMillis(); // Swap out all sessions idle longer than maxIdleSwap - // FIXME: What's preventing us from mangling a session during - // a request? if (maxIdleSwap >= 0) { for (int i = 0; i < sessions.length; i++) { StandardSession session = (StandardSession) sessions[i]; - if (!session.isValid()) - continue; - int timeIdle = // Truncate, do not round up - (int) ((timeNow - session.getLastAccessedTime()) / 1000L); - if (timeIdle > maxIdleSwap && timeIdle > minIdleSwap) { - if (log.isDebugEnabled()) - log.debug(sm.getString - ("persistentManager.swapMaxIdle", - session.getIdInternal(), new Integer(timeIdle))); - try { - swapOut(session); - } catch (IOException e) { - ; // This is logged in writeSession() + synchronized (session) { + if (!session.isValid()) + continue; + int timeIdle = // Truncate, do not round up + (int) ((timeNow - session.getLastAccessedTime()) / 1000L); + if (timeIdle > maxIdleSwap && timeIdle > minIdleSwap) { + if (log.isDebugEnabled()) + log.debug(sm.getString + ("persistentManager.swapMaxIdle", + session.getIdInternal(), new Integer(timeIdle))); + try { + swapOut(session); + } catch (IOException e) { + ; // This is logged in writeSession() + } } } } @@ -1073,19 +1090,21 @@ public abstract class PersistentManagerBase long timeNow = System.currentTimeMillis(); for (int i = 0; i < sessions.length && toswap > 0; i++) { - int timeIdle = // Truncate, do not round up - (int) ((timeNow - sessions[i].getLastAccessedTime()) / 1000L); - if (timeIdle > minIdleSwap) { - if(log.isDebugEnabled()) - log.debug(sm.getString - ("persistentManager.swapTooManyActive", - sessions[i].getIdInternal(), new Integer(timeIdle))); - try { - swapOut(sessions[i]); - } catch (IOException e) { - ; // This is logged in writeSession() + synchronized (sessions[i]) { + int timeIdle = // Truncate, do not round up + (int) ((timeNow - sessions[i].getLastAccessedTime()) / 1000L); + if (timeIdle > minIdleSwap) { + if(log.isDebugEnabled()) + log.debug(sm.getString + ("persistentManager.swapTooManyActive", + sessions[i].getIdInternal(), new Integer(timeIdle))); + try { + swapOut(sessions[i]); + } catch (IOException e) { + ; // This is logged in writeSession() + } + toswap--; } - toswap--; } } @@ -1107,20 +1126,22 @@ public abstract class PersistentManagerBase if (maxIdleBackup >= 0) { for (int i = 0; i < sessions.length; i++) { StandardSession session = (StandardSession) sessions[i]; - if (!session.isValid()) - continue; - int timeIdle = // Truncate, do not round up - (int) ((timeNow - session.getLastAccessedTime()) / 1000L); - if (timeIdle > maxIdleBackup) { - if (log.isDebugEnabled()) - log.debug(sm.getString - ("persistentManager.backupMaxIdle", - session.getIdInternal(), new Integer(timeIdle))); - - try { - writeSession(session); - } catch (IOException e) { - ; // This is logged in writeSession() + synchronized (session) { + if (!session.isValid()) + continue; + int timeIdle = // Truncate, do not round up + (int) ((timeNow - session.getLastAccessedTime()) / 1000L); + if (timeIdle > maxIdleBackup) { + if (log.isDebugEnabled()) + log.debug(sm.getString + ("persistentManager.backupMaxIdle", + session.getIdInternal(), new Integer(timeIdle))); + + try { + writeSession(session); + } catch (IOException e) { + ; // This is logged in writeSession() + } } } } diff --git a/java/org/apache/catalina/valves/PersistentValve.java b/java/org/apache/catalina/valves/PersistentValve.java index 1b7da1eb0..bb072a191 100644 --- a/java/org/apache/catalina/valves/PersistentValve.java +++ b/java/org/apache/catalina/valves/PersistentValve.java @@ -30,16 +30,18 @@ import org.apache.catalina.Session; import org.apache.catalina.Store; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; -import org.apache.catalina.core.StandardHost; import org.apache.catalina.session.PersistentManager; import org.apache.catalina.util.StringManager; /** - * Valve that implements the default basic behavior for the - * StandardHost container implementation. + * Valve that implements per-request session persistence. It is intended to be + * used with non-sticky load-balancers. *

* USAGE CONSTRAINT: To work correctly it requires a PersistentManager. + *

+ * USAGE CONSTRAINT: To work correctly it assumes only one request exists + * per session at any one time. * * @author Jean-Frederic Clere * @version $Revision$ $Date$ @@ -97,7 +99,6 @@ public class PersistentValve throws IOException, ServletException { // Select the Context to be used for this Request - StandardHost host = (StandardHost) getContainer(); Context context = request.getContext(); if (context == null) { response.sendError