And finally, here are the patches against 5.5.6 (same as HEAD at this moment in this case).

--
Jess Holle

Jess Holle wrote:

Here's the patches against 5.5.4....

Jess Holle wrote:

For those who tried finding through the patches only to find that they did not properly map against CVS (as I just used 'diff -u' in a directory not in a proper CVS tool...), please see the attached patch files against 5.0.30 (which were donoe with a proper CVS tool).

I have reworked this for 5.5.4 as well, but there are a number of changes between 5.5.4 and 5.5.6 in these same files. I thus plan to merge up to 5.5.6 before passing these patches along. If anyone is desparately interested in the 5.5.4 stuff now, let me know.

Overall, I do believe these are a substantial improvement over the current code, so I'd appreciate it if someone reviewed them a bit -- especially the 5.5.6 versions as it would be good to see this merged in to 5.5.x at least.

--
Jess Holle

Jess Holle wrote:

A week or two ago I found I had need of the persistent session manager, PersistentManagerBase -- and also noticed its experimental status.

Looking at the sources I found "FIXME" comments regarding: (1) a race condition between session passivation and session usage and (2) a lack of LRU sorting to passivate oldest sessions first. I also discovered that all passivated sessions are regularly (every minuted by default) read back into memory in their entirety to check if they should be expired.

The attached set of patches is intended to address all of these issues -- and seems to do so to the best of my (admittedly limited) testing.

There are additional fixes that should be made to JDBCStore (i.e. in general it seems to a trip to the database for every row in many cases where 1 per 'n' would suffice), but this was of less interest to me for the time being than FileStore, so I have not pursued these. Also, I introduced a new attribute to PersistentManagerBase, but have not yet exposed it via JMX. This is intentional at this point as I am uncertain as to the merits of the non-default value of this operation as of yet.

Comments and questions are welcome. All of the patches are against 5.0.30, but I could update them for 5.5.x if there was sufficient interest (e.g. if a committer was interested in committing them).

--
Jess Holle
[EMAIL PROTECTED]


Index: Request.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/connector/Request.java,v
retrieving revision 1.18
diff -u -r1.18 Request.java
--- Request.java        20 Nov 2004 21:10:47 -0000      1.18
+++ Request.java        16 Dec 2004 17:47:20 -0000
@@ -2178,11 +2178,11 @@
             } catch (IOException e) {
                 session = null;
             }
-            if ((session != null) && !session.isValid())
-                session = null;
             if (session != null) {
                 session.access();
-                return (session);
+                if (session.isValid())  // check isValid() *after* access()!
+                    return (session);
+                session = null;
             }
         }
 
Index: JDBCStore.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/JDBCStore.java,v
retrieving revision 1.12
diff -u -r1.12 JDBCStore.java
--- JDBCStore.java      2 Nov 2004 20:05:12 -0000       1.12
+++ JDBCStore.java      16 Dec 2004 17:47:20 -0000
@@ -151,6 +151,11 @@
     protected PreparedStatement preparedKeysSql = null;
 
     /**
+     * Variable to hold the <code>keysThatMayBeExpired()</code> prepared 
statement.
+     */
+    protected PreparedStatement preparedKeysThatMayBeExpiredSql = null;
+    
+    /**
      * Variable to hold the <code>save()</code> prepared statement.
      */
     protected PreparedStatement preparedSaveSql = null;
@@ -456,7 +461,7 @@
                                 + sessionTable + " WHERE " + sessionAppCol
                                 + " = ?";
                         preparedKeysSql = _conn.prepareStatement(keysSql);
-                                       }
+                    }
 
                     preparedKeysSql.setString(1, getName());
                     rst = preparedKeysSql.executeQuery();
@@ -491,6 +496,66 @@
         return (keys);
     }
 
+    protected String[] keysThatMayBeExpired()
+        throws IOException
+    {
+        int maxInactiveInterval = ((ManagerBase)manager).maxInactiveInterval;
+        if ( maxInactiveInterval < 0 )
+            return new String[0];
+        
+        ResultSet rst = null;
+        String keys[] = null;
+        synchronized (this) {
+            int numberOfTries = 2;
+            while (numberOfTries > 0) {
+
+                Connection _conn = getConnection();
+                if (_conn == null) {
+                    return (new String[0]);
+                }
+                try {
+                    if (preparedKeysThatMayBeExpiredSql == null) {
+                        String keysSql =
+                                "SELECT " + sessionIdCol + " FROM " + 
sessionTable +
+                                " WHERE " + sessionAppCol + " = ? and " +
+                                            sessionLastAccessedCol + " <= ?";
+                        preparedKeysThatMayBeExpiredSql = 
_conn.prepareStatement(keysSql);
+                    }
+
+                    preparedKeysThatMayBeExpiredSql.setString(1, getName());
+                    preparedKeysThatMayBeExpiredSql.setLong(2, 
System.currentTimeMillis() - maxInactiveInterval*1000L );
+                    rst = preparedKeysThatMayBeExpiredSql.executeQuery();
+                    ArrayList tmpkeys = new ArrayList();
+                    if (rst != null) {
+                        while (rst.next()) {
+                            tmpkeys.add(rst.getString(1));
+                        }
+                    }
+                    keys = (String[]) tmpkeys.toArray(new 
String[tmpkeys.size()]);
+                } catch (SQLException e) {
+                    
manager.getContainer().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
+                    keys = new String[0];
+                    // Close the connection so that it gets reopened next time
+                    if (dbConnection != null)
+                        close(dbConnection);
+                } finally {
+                    try {
+                        if (rst != null) {
+                            rst.close();
+                        }
+                    } catch (SQLException e) {
+                        ;
+                    }
+
+                    release(_conn);
+                }
+                numberOfTries--;
+            }
+        }
+
+        return (keys);
+    }
+
     /**
      * Return an integer containing a count of all Sessions
      * currently saved in this Store.  If there are no Sessions,
@@ -517,7 +582,7 @@
                                 + ") FROM " + sessionTable + " WHERE "
                                 + sessionAppCol + " = ?";
                         preparedSizeSql = _conn.prepareStatement(sizeSql);
-                                       }
+                                               }
 
                     preparedSizeSql.setString(1, getName());
                     rst = preparedSizeSql.executeQuery();
@@ -562,7 +627,7 @@
         ObjectInputStream ois = null;
         BufferedInputStream bis = null;
         Container container = manager.getContainer();
- 
+
         synchronized (this) {
             int numberOfTries = 2;
             while (numberOfTries > 0) {
@@ -607,7 +672,7 @@
                         _session = (StandardSession) 
manager.createEmptySession();
                         _session.readObjectData(ois);
                         _session.setManager(manager);
-                      } else if 
(manager.getContainer().getLogger().isDebugEnabled()) {
+                    } else if 
(manager.getContainer().getLogger().isDebugEnabled()) {
                         
manager.getContainer().getLogger().debug(getStoreName() + ": No persisted data 
object found");
                     }
                 } catch (SQLException e) {
@@ -765,15 +830,15 @@
                            + ", " + sessionMaxInactiveCol + ", "
                            + sessionLastAccessedCol
                            + ") VALUES (?, ?, ?, ?, ?, ?)";
-                       preparedSaveSql = _conn.prepareStatement(saveSql);
-                                       }
+                        preparedSaveSql = _conn.prepareStatement(saveSql);
+                    }
 
                     preparedSaveSql.setString(1, session.getId());
                     preparedSaveSql.setString(2, getName());
                     preparedSaveSql.setBinaryStream(3, in, size);
                     preparedSaveSql.setString(4, session.isValid() ? "1" : 
"0");
                     preparedSaveSql.setInt(5, 
session.getMaxInactiveInterval());
-                    preparedSaveSql.setLong(6, session.getLastAccessedTime());
+                    preparedSaveSql.setLong(6, 
((StandardSession)session).thisAccessedTime);
                     preparedSaveSql.execute();
                 } catch (SQLException e) {
                     
manager.getContainer().getLogger().error(sm.getString(getStoreName() + 
".SQLException", e));
@@ -909,8 +974,10 @@
         } catch (Throwable f) {
             ;
         }
-         
-               try {
+        // Removed in previous version.  Why?
+        // this.preparedClearSql = null;
+
+        try {
             preparedRemoveSql.close();
         } catch (Throwable f) {
             ;
Index: ManagerBase.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/ManagerBase.java,v
retrieving revision 1.37
diff -u -r1.37 ManagerBase.java
--- ManagerBase.java    22 Nov 2004 14:50:23 -0000      1.37
+++ ManagerBase.java    16 Dec 2004 17:47:20 -0000
@@ -662,7 +662,7 @@
         processingTime += ( timeEnd - timeNow );
 
     }
-
+    
     public void destroy() {
         if( oname != null )
             Registry.getRegistry(null, null).unregisterComponent(oname);
@@ -760,6 +760,32 @@
      */
     public Session createEmptySession() {
         return (getNewSession());
+    }
+
+
+    /**
+     * Allow manager to adjust session prior to normal session.access() 
operation.
+     * <p>
+     * This can be used by subclasses to reload session if it was swapped out
+     * due to race condition.
+     *
+     * @param session Session that is being accessed.
+     */
+    protected void preAccess( Session session )
+    {
+      // do nothing
+    }
+
+    /**
+     * Signal to Manager that session has been accessed.
+     * <p>
+     * This can be used by subclasses to maintain MRU/LRU data on
+     * sessions.
+     *
+     * @param session Session that is being accessed.
+     */
+    protected void access( Session session ) {
+        // do nothing here...
     }
 
 
Index: PersistentManagerBase.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/PersistentManagerBase.java,v
retrieving revision 1.25
diff -u -r1.25 PersistentManagerBase.java
--- PersistentManagerBase.java  22 Nov 2004 16:35:18 -0000      1.25
+++ PersistentManagerBase.java  16 Dec 2004 17:47:20 -0000
@@ -20,6 +20,11 @@
 import java.beans.PropertyChangeEvent;
 import java.beans.PropertyChangeListener;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import java.security.AccessController;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
@@ -204,17 +209,44 @@
     protected long processingTime = 0;
 
 
-    // ------------------------------------------------------------- Properties
+    /**
+     * Whether session should be synchronously swapped out immediately upon
+     * exceeding maxActiveSessions.  Defaults to 'false'.
+     */
+    protected boolean immediateSwapOnMaxActiveExceeded = false;
+    
+    
+    // ------------------------------------------------------------- 
Constructors
+
+
+    public PersistentManagerBase() {
+        sessions = new LinkedHashMap() {
+            protected boolean removeEldestEntry(Map.Entry eldest) {
+                if ( immediateSwapOnMaxActiveExceeded ) {
+                  int  maxActiveSessions = getMaxActiveSessions();
+                  if ( maxActiveSessions >= 0 )
+                      if ( size() > maxActiveSessions )
+                          if ( isStarted() )
+                              // try to swap out oldest entry; if entry is 
still too fresh then processMaxActiveSwaps() will swap out sessions when 
possible in the background
+                              swapOutSessionIfOldEnough( (StandardSession) 
eldest.getValue(), System.currentTimeMillis(), minIdleSwap, 
"persistentManager.swapTooManyActive" );
+                }
+                return ( false );
+            }
+        };
+    }
 
     
-  
+    // ------------------------------------------------------------- Properties
+
+
+
 
 
     /**
-        * Indicates how many seconds old a session can get, after its last use 
in a
-        * request, before it should be backed up to the store. -1 means 
sessions
-        * are not backed up.
-        */
+     * Indicates how many seconds old a session can get, after its last use in 
a
+     * request, before it should be backed up to the store. -1 means sessions
+     * are not backed up.
+     */
     public int getMaxIdleBackup() {
 
         return maxIdleBackup;
@@ -314,13 +346,13 @@
 
 
     /**
-        * Set the Container with which this Manager has been associated. If it 
is a
-        * Context (the usual case), listen for changes to the session timeout
-        * property.
-        * 
-        * @param container
-        *            The associated Container
-        */
+     * Set the Container with which this Manager has been associated. If it is 
a
+     * Context (the usual case), listen for changes to the session timeout
+     * property.
+     * 
+     * @param container
+     *            The associated Container
+     */
     public void setContainer(Container container) {
 
         // De-register from the old Container (if any)
@@ -500,6 +532,23 @@
     }
 
 
+    public boolean isImmediateSwapOnMaxActiveExceeded() {
+        return immediateSwapOnMaxActiveExceeded;
+    }
+    
+
+    
+    public void  setImmediateSwapOnMaxActiveExceeded( boolean 
immediateSwapOnMaxActiveExceeded ) {
+        if ( this.immediateSwapOnMaxActiveExceeded == 
immediateSwapOnMaxActiveExceeded )
+          return;
+        boolean oldImmediateSwapOnMaxActiveExceeded  = 
this.immediateSwapOnMaxActiveExceeded ;
+        this.immediateSwapOnMaxActiveExceeded  = 
immediateSwapOnMaxActiveExceeded ;
+        support.firePropertyChange("immediateSwapOnMaxActiveExceeded ",
+                                   new 
Boolean(oldImmediateSwapOnMaxActiveExceeded ),
+                                   new 
Boolean(this.immediateSwapOnMaxActiveExceeded ));
+    }    
+    
+    
     // --------------------------------------------------------- Public Methods
 
 
@@ -534,8 +583,8 @@
     /**
      * Implements the Manager interface, direct call to processExpires and 
processPersistenceChecks
      */
-       public void processExpires() {
-               
+    public void processExpires() {
+        
         long timeNow = System.currentTimeMillis();
         Session sessions[] = findSessions();
         int expireHere = 0 ;
@@ -556,8 +605,8 @@
         if(log.isDebugEnabled())
              log.debug("End expire sessions " + getName() + " processingTime " 
+ (timeEnd - timeNow) + " expired sessions: " + expireHere);
         processingTime += (timeEnd - timeNow);
-               
-       }
+         
+    }
 
 
     /**
@@ -608,6 +657,52 @@
         super.remove (session);
     }
 
+    /** If upon accessing a session we find it initially invalid see if it was
+     *  just swapped out.  If so, attempt to read the data back into this
+     *  session object as someone got a copy prior to the swapOut.
+     *
+     * @param session Session that is being accessed.
+     */
+    protected void preAccess( Session session )
+    {
+      StandardSession  stdSession = (StandardSession) session;
+      if ( stdSession.isValid || stdSession.expiring )
+        return;
+      String  id = stdSession.getId();
+      StandardSession  savedSession = null;
+      try
+      {
+        savedSession = (StandardSession) swapInWithoutAdd( id );
+      }
+      catch ( IOException e )
+      {
+        if (log.isDebugEnabled())
+            log.debug("Load from store failed for " + id );
+      }
+      if ( savedSession != null )
+      {
+        stdSession.copyFieldsOf( savedSession );
+        addSwappedInSession( stdSession );
+        savedSession.recycle();
+      }
+    }
+        
+    /**
+     * Signal to Manager that session has been accessed.
+     * <p>
+     * This is used here to maintain MRU/LRU data on sessions.
+     *
+     * @param session Session that is being accessed.
+     */
+    protected void  access( Session session ) {
+        synchronized (sessions) {
+            // pull session out of map and put it back in, thus sorting it
+            String sid = session.getId();
+            sessions.remove(sid);
+            sessions.put(sid, session);
+        }
+    }
+
     /**
      * Load all sessions found in the persistence mechanism, assuming
      * they are marked as valid and have not passed their expiration
@@ -660,6 +755,39 @@
                 log.error("Failed load session from store, " + e.getMessage(), 
e);
             }
 
+        sortSessionsByAccessTimes();
+    }
+
+
+    /** Sort sessions by access times, i.e. during load().
+     */
+    private void sortSessionsByAccessTimes() {
+        ArrayList sessionList;
+        {
+            Session  sessionArr[] = findSessions();
+            sessionList = new ArrayList( sessionArr.length );
+            for ( int i = 0; i < sessionArr.length; ++i )
+              sessionList.add( sessionArr[i] );
+        }
+        Collections.sort( sessionList,
+            new Comparator() {
+                public int compare(Object o1, Object o2) {
+                    StandardSession  session1 = (StandardSession) o1;
+                    StandardSession  session2 = (StandardSession) o2;
+                    long  accessTime1 = session1.thisAccessedTime;
+                    long  accessTime2 = session2.thisAccessedTime;
+                    if ( accessTime1 < accessTime2 )
+                        return -1;
+                    if ( accessTime1 == accessTime2 )
+                        return 0;
+                    // if ( accessTime1 > accessTime2 )
+                        return 1;
+                }
+            }
+        );
+        int  nSessions = sessionList.size();
+        for ( int i = 0; i < nSessions; ++i )
+            access( (Session) sessionList.get( i ) );
     }
 
 
@@ -749,6 +877,13 @@
      * is invalid or past its expiration.
      */
     protected Session swapIn(String id) throws IOException {
+        Session  session = swapInWithoutAdd( id );
+        if ( session != null )
+          addSwappedInSession( session );
+        return ( session );
+    }
+    
+    protected Session swapInWithoutAdd(String id) throws IOException {
 
         if (store == null)
             return null;
@@ -791,15 +926,18 @@
         if(log.isDebugEnabled())
             log.debug(sm.getString("persistentManager.swapIn", id));
 
+        return (session);
+
+    }
+
+    protected void  addSwappedInSession( Session session )
+    {
         session.setManager(this);
         // make sure the listeners know about it.
         ((StandardSession)session).tellNew();
         add(session);
         ((StandardSession)session).activate();
         session.endAccess();
-
-        return (session);
-
     }
 
 
@@ -1023,29 +1161,10 @@
         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.getId(), new Integer(timeIdle)));
-                    try {
-                        swapOut(session);
-                    } catch (IOException e) {
-                        ;   // This is logged in writeSession()
-                    }
-                }
-            }
-        }
-
+        int  minSwapSecs = ( ( maxIdleSwap > minIdleSwap ) ? maxIdleSwap : 
minIdleSwap );
+        for (int i = sessions.length - 1; i >= 0; --i)
+            swapOutSessionIfOldEnough( (StandardSession) sessions[i], timeNow, 
minSwapSecs, "persistentManager.swapMaxIdle" );
+            
     }
 
 
@@ -1059,7 +1178,6 @@
 
         Session sessions[] = findSessions();
 
-        // FIXME: Smarter algorithm (LRU)
         if (getMaxActiveSessions() >= sessions.length)
             return;
 
@@ -1071,24 +1189,36 @@
         int toswap = sessions.length - getMaxActiveSessions();
         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].getId(), new Integer(timeIdle)));
-                try {
-                    swapOut(sessions[i]);
-                } catch (IOException e) {
-                    ;   // This is logged in writeSession()
-                }
-                toswap--;
-            }
-        }
+        for (int i = sessions.length - 1; i >= 0 && toswap > 0; --i)
+            if ( swapOutSessionIfOldEnough( (StandardSession) sessions[i], 
timeNow, minIdleSwap, "persistentManager.swapTooManyActive" ) )
+              --toswap;
 
     }
+    
+    
+    private boolean  swapOutSessionIfOldEnough( StandardSession session, long 
timeNow, int minSwapSecs, String debugString ) {
+        synchronized ( session ) {
+          int timeIdle = // Truncate, do not round up
+              (int) ((timeNow - session.thisAccessedTime) / 1000L);
+          if (timeIdle > minSwapSecs ) {
+              if ( session.accessCount > 0 ) {
+                  if (log.isDebugEnabled())
+                      log.debug( 
"PersistentManagerBase.swapOutSessionIfOldEnough(): session [" + 
session.getId() +
+                                 "] access count [" + session.accessCount + "] 
> 0 " );
+                  return ( false );  // session may still be being accessed
+              }
+              if(log.isDebugEnabled())
+                  log.debug(sm.getString(debugString, session.getId(), new 
Integer(timeIdle)));
+              try {
+                  swapOut(session);
+              } catch (IOException e) {
+                  ;   // This is logged in writeSession()
+              }
+              return ( true );
+          }
+        }
+        return ( false );
+    }
 
 
     /**
@@ -1104,22 +1234,32 @@
 
         // Back up all sessions idle longer than maxIdleBackup
         if (maxIdleBackup >= 0) {
-            for (int i = 0; i < sessions.length; i++) {
+            for (int i = sessions.length - 1; i >= 0; --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.getId(), 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.thisAccessedTime) / 1000L);
+                    if (timeIdle > maxIdleBackup) {
+                        if ( session.accessCount > 0 ) {
+                            if (log.isDebugEnabled())
+                                log.debug( 
"PersistentManagerBase.processMaxIdleBackups(): session [" + session.getId() +
+                                           "] access count [" + 
session.accessCount + "] > 0 " );
+                            continue;  // session may still be being accessed
+                        }
+                        
+                        // TO_DO: enhance store to allow quick 
'if-modified-since' sort of check here!
+                        if (log.isDebugEnabled())
+                            log.debug(sm.getString
+                                ("persistentManager.backupMaxIdle",
+                                session.getId(), new Integer(timeIdle)));
+                        try {
+                            writeSession(session);
+                        } catch (IOException e) {
+                            ;   // This is logged in writeSession()
+                        }
                     }
                 }
             }
Index: StandardSession.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/StandardSession.java,v
retrieving revision 1.49
diff -u -r1.49 StandardSession.java
--- StandardSession.java        22 Nov 2004 16:35:18 -0000      1.49
+++ StandardSession.java        16 Dec 2004 17:47:20 -0000
@@ -582,7 +582,14 @@
      * should be called by the context when a request comes in for a particular
      * session, even if the application does not reference it.
      */
-    public void access() {
+    public synchronized void access() {
+
+        ManagerBase  managerBase = ( ( manager instanceof ManagerBase ) ? 
(ManagerBase) manager : null );
+        if ( managerBase != null )
+            managerBase.preAccess( this );
+        
+        if ( !isValid || expiring )
+          return;
 
         this.lastAccessedTime = this.thisAccessedTime;
         this.thisAccessedTime = System.currentTimeMillis();
@@ -590,7 +597,10 @@
         evaluateIfValid();
 
         accessCount++;
-
+        
+        if ( isValid )
+            if ( managerBase != null )
+                managerBase.access( this );
     }
 
 
@@ -600,8 +610,11 @@
     public void endAccess() {
 
         isNew = false;
-        accessCount--;
-
+        if ( accessCount > 0 )
+            --accessCount;
+        else if ( manager.getContainer().getLogger().isDebugEnabled() )
+            manager.getContainer().getLogger().debug(
+              "Session " + getId() + " accessCount [" + ( accessCount - 1 ) + 
"] < 0 " );  // show access count as it would be if we had decremented
     }
 
 
@@ -799,6 +812,20 @@
 
     }
 
+    /**
+     * Copy all non-transient fields other than id from 'otherSession' to
+     * this session.
+     */
+    protected void copyFieldsOf( StandardSession otherSession )
+    {
+      this.attributes = (HashMap) otherSession.attributes.clone();
+      this.creationTime = otherSession.creationTime ;
+      this.lastAccessedTime = otherSession.lastAccessedTime;
+      this.maxInactiveInterval = otherSession.maxInactiveInterval;
+      this.isNew = otherSession.isNew;
+      this.isValid = otherSession.isValid;
+      this.thisAccessedTime = otherSession.thisAccessedTime;
+    }
 
     /**
      * Release all object references, and initialize instance variables, in
@@ -811,7 +838,7 @@
         setAuthType(null);
         creationTime = 0L;
         expiring = false;
-        id = null;
+        // id = null;
         lastAccessedTime = 0L;
         maxInactiveInterval = -1;
         accessCount = 0;
@@ -819,7 +846,7 @@
         setPrincipal(null);
         isNew = false;
         isValid = false;
-        manager = null;
+        // manager = null;
 
     }
 
Index: StoreBase.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/StoreBase.java,v
retrieving revision 1.10
diff -u -r1.10 StoreBase.java
--- StoreBase.java      2 Nov 2004 19:07:51 -0000       1.10
+++ StoreBase.java      16 Dec 2004 17:47:20 -0000
@@ -175,29 +175,26 @@
         long timeNow = System.currentTimeMillis();
         String[] keys = null;
 
-         if(!started) {
+        if(!started) {
             return;
         }
 
         try {
-            keys = keys();
+            keys = keysThatMayBeExpired();
         } catch (IOException e) {
             manager.getContainer().getLogger().error("Error getting keys", e);
             return;
         }
         if (manager.getContainer().getLogger().isDebugEnabled()) {
             manager.getContainer().getLogger().debug(getStoreName()+ ": 
processExpires check number of " + keys.length + " sessions" );
-        }
-    
+        }    
+
         for (int i = 0; i < keys.length; i++) {
             try {
-                StandardSession session = (StandardSession) load(keys[i]);
+                StandardSession session = (StandardSession) 
loadSessionIfShouldBeExpired(keys[i]);
                 if (session == null) {
                     continue;
                 }
-                if (session.isValid()) {
-                    continue;
-                }
                 if (manager.getContainer().getLogger().isDebugEnabled()) {
                     manager.getContainer().getLogger().debug(getStoreName()+ 
": processExpires expire store session " + keys[i] );
                 }
@@ -220,6 +217,33 @@
         }
     }
 
+    protected String[] keysThatMayBeExpired()
+        throws IOException
+    {
+        return ( keys() );
+    }
+
+    protected StandardSession loadSessionIfShouldBeExpired( String sessionId )
+    {
+        StandardSession session;
+        try
+        {
+            session = (StandardSession) load(sessionId);
+        }
+        catch ( ClassNotFoundException e )
+        {
+            session = null;
+        }
+        catch ( IOException e )
+        {
+            session = null;
+        }
+        if ( session != null )
+            if (session.isValid())
+                return ( null );
+        return ( session );
+    }
+    
 
     // --------------------------------------------------------- Thread Methods
 
Index: FileStore.java
===================================================================
RCS file: 
/home/cvspublic/jakarta-tomcat-catalina/catalina/src/share/org/apache/catalina/session/FileStore.java,v
retrieving revision 1.6
diff -u -r1.6 FileStore.java
--- FileStore.java      17 Nov 2004 15:00:48 -0000      1.6
+++ FileStore.java      16 Dec 2004 17:47:20 -0000
@@ -373,8 +373,31 @@
             oos.close();
         }
 
+        // give file modification date to match last session access time
+        if ( !file.setLastModified( 
((StandardSession)session).thisAccessedTime ) )
+               if (manager.getContainer().getLogger().isDebugEnabled())
+                   manager.getContainer().getLogger().debug("Could not set 
last modified date on file store for " + session.getId() );
+
     }
 
+    // use file modification date to check if we should bother loading session
+    protected StandardSession loadSessionIfShouldBeExpired( String sessionId )
+    {
+        File file = file(sessionId);
+        if (file != null) {
+            long sessionAccessTime = file.lastModified();
+            if ( sessionAccessTime > 0 ) {
+                int  maxInactiveInterval = 
((ManagerBase)manager).maxInactiveInterval;
+                if ( maxInactiveInterval >= 0 ) { 
+                    long timeNow = System.currentTimeMillis();
+                    int timeIdle = (int) ((timeNow - sessionAccessTime) / 
1000L);
+                    if (timeIdle >= maxInactiveInterval)
+                        return ( super.loadSessionIfShouldBeExpired( sessionId 
) );
+                }
+            }
+        }
+        return ( null );
+    }
 
     // -------------------------------------------------------- Private Methods
 

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to