tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r656751 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/catalina/session/PersistentManagerBase.java java/org/apache/catalina/valves/PersistentValve.java webapps/docs/changelog.xml
Date Thu, 15 May 2008 17:27:51 GMT
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message