Author: markt
Date: Wed Jul 1 13:19:15 2009
New Revision: 790155
URL: http://svn.apache.org/viewvc?rev=790155&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=43343
Port of r656751, r778523, r778524, r784453 and r784602
Because, unlike 6.0.x, accessCount is not atomic I made it volatile and sync'd
the updates.
Modified:
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/PersistentManagerBase.java
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/StandardSession.java
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/PersistentValve.java
tomcat/container/tc5.5.x/webapps/docs/changelog.xml
tomcat/container/tc5.5.x/webapps/docs/config/manager.xml
Modified:
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/PersistentManagerBase.java
URL:
http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/PersistentManagerBase.java?rev=790155&r1=790154&r2=790155&view=diff
==============================================================================
---
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/PersistentManagerBase.java
(original)
+++
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/PersistentManagerBase.java
Wed Jul 1 13:19:15 2009
@@ -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);
@@ -797,6 +814,9 @@
((StandardSession)session).tellNew();
add(session);
((StandardSession)session).activate();
+ // endAccess() to ensure timeouts happen correctly.
+ // access() to keep access count correct or it will end up negative
+ session.access();
session.endAccess();
return (session);
@@ -1023,24 +1043,28 @@
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 (session.accessCount > 0) {
+ // Session is currently being accessed - skip it
+ continue;
+ }
+ 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()
+ }
}
}
}
@@ -1072,19 +1096,26 @@
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()
+ StandardSession session = (StandardSession) sessions[i];
+ synchronized (session) {
+ int timeIdle = // Truncate, do not round up
+ (int) ((timeNow - session.getLastAccessedTime()) / 1000L);
+ if (timeIdle > minIdleSwap) {
+ if (session.accessCount > 0) {
+ // Session is currently being accessed - skip it
+ continue;
+ }
+ if(log.isDebugEnabled())
+ log.debug(sm.getString
+ ("persistentManager.swapTooManyActive",
+ session.getIdInternal(), new Integer(timeIdle)));
+ try {
+ swapOut(session);
+ } catch (IOException e) {
+ // This is logged in writeSession()
+ }
+ toswap--;
}
- toswap--;
}
}
@@ -1106,20 +1137,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/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/StandardSession.java
URL:
http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/StandardSession.java?rev=790155&r1=790154&r2=790155&view=diff
==============================================================================
---
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/StandardSession.java
(original)
+++
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/session/StandardSession.java
Wed Jul 1 13:19:15 2009
@@ -273,7 +273,7 @@
/**
* The access count for this session.
*/
- protected transient int accessCount = 0;
+ protected volatile transient int accessCount = 0;
private Object lock = new Object();
@@ -711,7 +711,9 @@
}
}
}
- accessCount = 0;
+ synchronized(lock) {
+ accessCount = 0;
+ }
setValid(false);
/*
@@ -851,7 +853,9 @@
id = null;
lastAccessedTime = 0L;
maxInactiveInterval = -1;
- accessCount = 0;
+ synchronized(lock) {
+ accessCount = 0;
+ }
notes.clear();
setPrincipal(null);
isNew = false;
Modified:
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/PersistentValve.java
URL:
http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/PersistentValve.java?rev=790155&r1=790154&r2=790155&view=diff
==============================================================================
---
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/PersistentValve.java
(original)
+++
tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/valves/PersistentValve.java
Wed Jul 1 13:19:15 2009
@@ -35,10 +35,13 @@
/**
- * 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$
@@ -134,6 +137,7 @@
manager.add(session);
// ((StandardSession)session).activate();
session.access();
+ session.endAccess();
}
}
}
Modified: tomcat/container/tc5.5.x/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/webapps/docs/changelog.xml?rev=790155&r1=790154&r2=790155&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/webapps/docs/changelog.xml (original)
+++ tomcat/container/tc5.5.x/webapps/docs/changelog.xml Wed Jul 1 13:19:15 2009
@@ -96,6 +96,10 @@
<bug>42707</bug>: Make adding a host alias via JMX take effect
immediately. (markt)
</fix>
+ <fix>
+ <bug>43343</bug>: Correctly handle requesting a session we are in the
+ middle of persisting. Based on a suggestion by Wade Chandler. (markt)
+ </fix>
<add>
<bug>44382</bug>: Add support for using httpOnly for session cookies.
This is disabled by default. (markt/fhanik)
Modified: tomcat/container/tc5.5.x/webapps/docs/config/manager.xml
URL:
http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/webapps/docs/config/manager.xml?rev=790155&r1=790154&r2=790155&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/webapps/docs/config/manager.xml (original)
+++ tomcat/container/tc5.5.x/webapps/docs/config/manager.xml Wed Jul 1
13:19:15 2009
@@ -164,7 +164,12 @@
<p><em><strong>WARNING - Use of this Manager implementation
has not been thoroughly tested, and should be considered experimental!
</strong></em></p>
-
+
+ <p><strong>NOTE:</strong> You must set the
+ <code>org.apache.catalina.STRICT_SERVLET_COMPLIANCE</code>
+ <a href="systemprops.html">system property</a> to <code>true</code> for
+ the persistent manager to work correctly.</p>
+
<p>The persistent implementation of <strong>Manager</strong> is
<strong>org.apache.catalina.session.PersistentManager</strong>. In
addition to the usual operations of creating and deleting sessions, a
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
