tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r790155 - in /tomcat/container/tc5.5.x: catalina/src/share/org/apache/catalina/session/ catalina/src/share/org/apache/catalina/valves/ webapps/docs/ webapps/docs/config/
Date Wed, 01 Jul 2009 13:19:16 GMT
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message