Author: markt
Date: Thu Sep 5 15:42:48 2013
New Revision: 1520349
URL: http://svn.apache.org/r1520349
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/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
tomcat/trunk/java/org/apache/catalina/session/StandardSession.java
Modified: tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java?rev=1520349&r1=1520348&r2=1520349&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/ha/session/DeltaSession.java Thu Sep
5 15:42:48 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/trunk/java/org/apache/catalina/session/StandardSession.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/StandardSession.java?rev=1520349&r1=1520348&r2=1520349&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/StandardSession.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/session/StandardSession.java Thu Sep
5 15:42:48 2013
@@ -658,14 +658,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;
}
@@ -683,7 +683,7 @@ public class StandardSession implements
}
}
- return (this.isValid);
+ return this.isValid;
}
@@ -777,14 +777,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)
@@ -860,7 +862,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);
@@ -883,6 +884,7 @@ public class StandardSession implements
}
// We have completed expire of this session
+ setValid(false);
expiring = false;
// Unbind any objects associated with this session
@@ -1563,7 +1565,7 @@ public class StandardSession implements
* check.
*/
protected boolean isValidInternal() {
- return (this.isValid || this.expiring);
+ return this.isValid;
}
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
