Return-Path: Delivered-To: apmail-jakarta-tomcat-dev-archive@www.apache.org Received: (qmail 61637 invoked from network); 16 Dec 2004 17:55:05 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur-2.apache.org with SMTP; 16 Dec 2004 17:55:05 -0000 Received: (qmail 58499 invoked by uid 500); 16 Dec 2004 17:54:57 -0000 Delivered-To: apmail-jakarta-tomcat-dev-archive@jakarta.apache.org Received: (qmail 58475 invoked by uid 500); 16 Dec 2004 17:54:57 -0000 Mailing-List: contact tomcat-dev-help@jakarta.apache.org; run by ezmlm Precedence: bulk List-Unsubscribe: List-Subscribe: List-Help: List-Post: List-Id: "Tomcat Developers List" Reply-To: "Tomcat Developers List" Delivered-To: mailing list tomcat-dev@jakarta.apache.org Received: (qmail 58461 invoked by uid 99); 16 Dec 2004 17:54:56 -0000 X-ASF-Spam-Status: No, hits=0.4 required=10.0 tests=DNS_FROM_RFC_ABUSE X-Spam-Check-By: apache.org Received-SPF: pass (hermes.apache.org: local policy) Received: from relay2.ptc.com (HELO relay2.ptc.com) (12.11.148.122) by apache.org (qpsmtpd/0.28) with ESMTP; Thu, 16 Dec 2004 09:52:09 -0800 Received: from hq-exfe4.ptcnet.ptc.com (132.253.201.83) by relay2.ptc.com with ESMTP; 16 Dec 2004 12:51:18 -0500 X-IronPort-AV: i="3.87,147,1099285200"; d="scan'208"; a="5034081:sNHT49732720" Received: from [132.253.10.127] ([132.253.10.127]) by HQ-EXFE4.ptcnet.ptc.com with Microsoft SMTPSVC(5.0.2195.6713); Thu, 16 Dec 2004 12:50:44 -0500 Message-ID: <41C1CAF4.7090506@ptc.com> Date: Thu, 16 Dec 2004 11:50:44 -0600 From: Jess Holle User-Agent: Mozilla Thunderbird 1.0 (Windows/20041206) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Jess Holle CC: Tomcat Developers List Subject: Re: PersistentManagerBase (etc) Patches References: <2129B7848043D411881A00B0D0627EFE01F6E037@exchange.sigaba.com> <01cb01c4dcb6$45ea2d30$cb37a8c0@bbarkerxp> <41BE6247.9000807@ptc.com> <41C085CF.5030401@ptc.com> <41C09E9A.2060608@ptc.com> In-Reply-To: <41C09E9A.2060608@ptc.com> Content-Type: multipart/mixed; boundary="------------090403070208030207070900" X-OriginalArrivalTime: 16 Dec 2004 17:50:44.0865 (UTC) FILETIME=[C472EF10:01C4E397] X-Virus-Checked: Checked X-Spam-Rating: minotaur-2.apache.org 1.6.2 0/1000/N --------------090403070208030207070900 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 >>> jessh@ptc.com >> --------------090403070208030207070900 Content-Type: text/plain; name="Request.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="Request.patch" 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; } } --------------090403070208030207070900 Content-Type: text/plain; name="JDBCStore.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="JDBCStore.patch" 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 keysThatMayBeExpired() prepared statement. + */ + protected PreparedStatement preparedKeysThatMayBeExpiredSql = null; + + /** * Variable to hold the save() 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) { ; --------------090403070208030207070900 Content-Type: text/plain; name="ManagerBase.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="ManagerBase.patch" 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. + *

+ * 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. + *

+ * 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... } --------------090403070208030207070900 Content-Type: text/plain; name="PersistentManagerBase.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="PersistentManagerBase.patch" 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. + *

+ * 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() + } } } } --------------090403070208030207070900 Content-Type: text/plain; name="StandardSession.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="StandardSession.patch" 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; } --------------090403070208030207070900 Content-Type: text/plain; name="StoreBase.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="StoreBase.patch" 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 --------------090403070208030207070900 Content-Type: text/plain; name="FileStore.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="FileStore.patch" 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 --------------090403070208030207070900 Content-Type: text/plain; charset=us-ascii --------------------------------------------------------------------- To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org --------------090403070208030207070900--