Author: markt
Date: Thu May 1 14:11:26 2008
New Revision: 652662
URL: http://svn.apache.org/viewvc?rev=652662&view=rev
Log:
Fix bug 43343. Correctly handle the case where a request arrives for a session
we are in the middle of persisting.
Modified:
tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
tomcat/trunk/java/org/apache/catalina/valves/PersistentValve.java
Modified:
tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java?rev=652662&r1=652661&r2=652662&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
Thu May 1 14:11:26 2008
@@ -590,6 +590,23 @@
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 @@
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 @@
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 @@
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()
+ }
}
}
}
Modified: tomcat/trunk/java/org/apache/catalina/valves/PersistentValve.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/PersistentValve.java?rev=652662&r1=652661&r2=652662&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/PersistentValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/valves/PersistentValve.java Thu May
1 14:11:26 2008
@@ -30,16 +30,18 @@
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
- * <code>StandardHost</code> container implementation.
+ * Valve that implements per-request session persistence. It is intended to be
+ * used with non-sticky load-balancers.
* <p>
* <b>USAGE CONSTRAINT</b>: To work correctly it requires a PersistentManager.
+ * <p>
+ * <b>USAGE CONSTRAINT</b>: 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 @@
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
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
