Author: markt
Date: Thu May 15 10:27:50 2008
New Revision: 656751
URL: http://svn.apache.org/viewvc?rev=656751&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=43343
Correctly handle requesting a session we are in the middle of persisting
Based on a suggestion by Wade Chandler
Modified:
tomcat/tc6.0.x/trunk/STATUS.txt
tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java
tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=656751&r1=656750&r2=656751&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Thu May 15 10:27:50 2008
@@ -51,14 +51,6 @@
+1: jfclere, rjung, fhanik, remm, pero
-1:
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=43343
- Correctly handle requesting a session we are in the middle of
- persisting
- http://svn.apache.org/viewvc?rev=652662&view=rev
- Based on a suggestion by Wade Chandler
- +1: markt, remm, pero
- -1:
-
* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=43333
Correct sendfile docs
http://svn.apache.org/viewvc?rev=652666&view=rev
Modified:
tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java?rev=656751&r1=656750&r2=656751&view=diff
==============================================================================
---
tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
(original)
+++
tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
Thu May 15 10:27:50 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/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java?rev=656751&r1=656750&r2=656751&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/valves/PersistentValve.java
Thu May 15 10:27:50 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
Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=656751&r1=656750&r2=656751&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Thu May 15 10:27:50 2008
@@ -47,6 +47,10 @@
using the webapp class loader when we create them. (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>
+ <fix>
<bug>43142</bug>: Don't assume a directory named xxx.war is a war file.
(markt)
</fix>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
