Author: markt Date: Thu Sep 5 16:13:06 2013 New Revision: 1520358 URL: http://svn.apache.org/r1520358 Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55521 Ensure that session.expire() doesn't return until the session has been invalidated. Ensure that the return valid of session.isValid() is consistent the current state.
Modified: tomcat/tc7.0.x/trunk/ (props changed) tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/StandardSession.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Propchange: tomcat/tc7.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1520349 Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java?rev=1520358&r1=1520357&r2=1520358&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java Thu Sep 5 16:13:06 2013 @@ -385,12 +385,12 @@ public class DeltaSession extends Standa */ @Override public boolean isValid() { - if (this.expiring) { - return true; - } if (!this.isValid) { return false; } + if (this.expiring) { + return true; + } if (ACTIVITY_CHECK && accessCount.get() > 0) { return true; } @@ -445,30 +445,49 @@ public class DeltaSession extends Standa } public void expire(boolean notify, boolean notifyCluster) { - if (expiring) + + // Check to see if session has already been invalidated. + // Do not check expiring at this point as expire should not return until + // isValid is false + if (!isValid) return; - String expiredId = getIdInternal(); - if(notifyCluster && expiredId != null && manager != null && - manager instanceof DeltaManager) { - DeltaManager dmanager = (DeltaManager)manager; - CatalinaCluster cluster = dmanager.getCluster(); - ClusterMessage msg = dmanager.requestCompleted(expiredId, true); - if (msg != null) { - cluster.send(msg); + synchronized (this) { + // Check again, now we are inside the sync so this code only runs once + // Double check locking - isValid needs to be volatile + if (!isValid) + return; + + if (manager == null) + return; + + // Mark this session as "being expired". The flag will be unset in + // the call to super.expire(notify) + expiring = true; + + String expiredId = getIdInternal(); + + if(notifyCluster && expiredId != null && + manager instanceof DeltaManager) { + DeltaManager dmanager = (DeltaManager)manager; + CatalinaCluster cluster = dmanager.getCluster(); + ClusterMessage msg = dmanager.requestCompleted(expiredId, true); + if (msg != null) { + cluster.send(msg); + } } - } - super.expire(notify); + super.expire(notify); - if (notifyCluster) { - if (log.isDebugEnabled()) - log.debug(sm.getString("deltaSession.notifying", - ((ClusterManager)manager).getName(), - Boolean.valueOf(isPrimarySession()), - expiredId)); - if ( manager instanceof DeltaManager ) { - ( (DeltaManager) manager).sessionExpired(expiredId); + if (notifyCluster) { + if (log.isDebugEnabled()) + log.debug(sm.getString("deltaSession.notifying", + ((ClusterManager)manager).getName(), + Boolean.valueOf(isPrimarySession()), + expiredId)); + if ( manager instanceof DeltaManager ) { + ( (DeltaManager) manager).sessionExpired(expiredId); + } } } } Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/StandardSession.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/StandardSession.java?rev=1520358&r1=1520357&r2=1520358&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/StandardSession.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/StandardSession.java Thu Sep 5 16:13:06 2013 @@ -634,14 +634,14 @@ public class StandardSession implements @Override public boolean isValid() { - if (this.expiring) { - return true; - } - if (!this.isValid) { return false; } + if (this.expiring) { + return true; + } + if (ACTIVITY_CHECK && accessCount.get() > 0) { return true; } @@ -659,7 +659,7 @@ public class StandardSession implements } } - return (this.isValid); + return this.isValid; } @@ -753,14 +753,16 @@ public class StandardSession implements */ public void expire(boolean notify) { - // Check to see if expire is in progress or has previously been called - if (expiring || !isValid) + // Check to see if session has already been invalidated. + // Do not check expiring at this point as expire should not return until + // isValid is false + if (!isValid) return; synchronized (this) { // Check again, now we are inside the sync so this code only runs once - // Double check locking - expiring and isValid need to be volatile - if (expiring || !isValid) + // Double check locking - isValid needs to be volatile + if (!isValid) return; if (manager == null) @@ -834,7 +836,6 @@ public class StandardSession implements if (ACTIVITY_CHECK) { accessCount.set(0); } - setValid(false); // Remove this session from our manager's active sessions manager.remove(this, true); @@ -857,6 +858,7 @@ public class StandardSession implements } // We have completed expire of this session + setValid(false); expiring = false; // Unbind any objects associated with this session @@ -1537,7 +1539,7 @@ public class StandardSession implements * check. */ protected boolean isValidInternal() { - return (this.isValid || this.expiring); + return this.isValid; } /** Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1520358&r1=1520357&r2=1520358&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Thu Sep 5 16:13:06 2013 @@ -155,6 +155,13 @@ (kfujino) </fix> <fix> + <bug>55521</bug>: Ensure that calls to + <code>HttpSession.invalidate()</code> do not return until the session + has been invalidated. Also ensure that checks on the validity of a + session return a result consistent with any previous call to + <code>HttpSession.invalidate()</code>. (markt) + </fix> + <fix> <bug>55524</bug>: Refactor to avoid a possible deadlock when handling an <code>IOException</code> during output when using Tomcat' proprietary (and deprecated) WebSocket API. (markt) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org