You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/09/05 18:13:07 UTC

svn commit: r1520358 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/ha/session/DeltaSession.java java/org/apache/catalina/session/StandardSession.java webapps/docs/changelog.xml

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&apos;
         proprietary (and deprecated) WebSocket API. (markt)



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org