[jboss-cvs] JBossAS SVN: r61911 - branches/JBoss_4_0_3_SP1_CP/tomcat/src/main/org/jboss/web/tomcat/tc5/sso.

[jboss-cvs-commits at lists.jboss.org]

Author: bstansberry at jboss.com
Date: 2007-03-30 15:27:59 -0400 (Fri, 30 Mar 2007)
New Revision: 61911

Modified:
   branches/JBoss_4_0_3_SP1_CP/tomcat/src/main/org/jboss/web/tomcat/tc5/sso/TreeCacheSSOClusterManager.java
Log:
[ASPATCH-157] Fix broken synchronization in TreeCacheSSOClusterManager

Modified: branches/JBoss_4_0_3_SP1_CP/tomcat/src/main/org/jboss/web/tomcat/tc5/sso/TreeCacheSSOClusterManager.java
===================================================================
--- branches/JBoss_4_0_3_SP1_CP/tomcat/src/main/org/jboss/web/tomcat/tc5/sso/TreeCacheSSOClusterManager.java	2007-03-30 19:13:04 UTC (rev 61910)
+++ branches/JBoss_4_0_3_SP1_CP/tomcat/src/main/org/jboss/web/tomcat/tc5/sso/TreeCacheSSOClusterManager.java	2007-03-30 19:27:59 UTC (rev 61911)
@@ -9,7 +9,6 @@
 import java.io.Serializable;
 import java.security.Principal;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.Set;
 
 import javax.management.MBeanServer;
@@ -91,20 +90,19 @@
    // -------------------------------------------------------  Instance Fields
    
    /**
-    * List of SSO ids which this object is currently storing to the cache
+    * SSO id which the thread is currently storing to the cache
     */
-   private LinkedList beingLocallyAdded = new LinkedList();
+   private ThreadLocal beingLocallyAdded = new ThreadLocal();
 
    /**
-    * List of SSO ids which this object is currently removing from the cache
+    * SSO id which a thread is currently removing from the cache
     */
-   private LinkedList beingLocallyRemoved = new LinkedList();
+   private ThreadLocal beingLocallyRemoved = new ThreadLocal();
 
    /**
-    * List of SSO ids which are being deregistered due to removal on another
-    * node
+    * SSO id which the thread is deregistering due to removal on another node
     */
-   private LinkedList beingRemotelyRemoved = new LinkedList();
+   private ThreadLocal beingRemotelyRemoved = new ThreadLocal();
 
    /**
     * ObjectName of the TreeCache
@@ -360,16 +358,14 @@
       }
       
       // Check whether we are already handling this removal 
-      //synchronized (beingLocallyRemoved)
+      if (ssoId.equals(beingLocallyRemoved.get()))
       {
-         if (beingLocallyRemoved.contains(ssoId))
-         {
-            return;
-         }         
-         // Add this SSO to our list of in-process local removals so
-         // this.nodeRemoved() will ignore the removal
-         beingLocallyRemoved.add(ssoId);
-      }
+         return;
+      }         
+      
+      // Add this SSO to our list of in-process local removals so
+      // this.nodeRemoved() will ignore the removal
+      beingLocallyRemoved.set(ssoId);
 
       if (log.isTraceEnabled())
       {
@@ -390,10 +386,7 @@
       }
       finally
       {
-         //synchronized (beingLocallyRemoved)
-         {
-            beingLocallyRemoved.remove(ssoId);
-         }
+         beingLocallyRemoved.set(null);
       }
    }
 
@@ -495,12 +488,9 @@
       
       // Check that this session removal is not due to our own deregistration
       // of an SSO following receipt of a nodeRemoved() call
-      //synchronized(beingRemotelyRemoved)
+      if (ssoId.equals(beingRemotelyRemoved.get()))
       {
-         if (beingRemotelyRemoved.contains(ssoId))
-         {
-            return;
-         }
+         return;
       }
 
       if (log.isTraceEnabled())
@@ -531,12 +521,8 @@
             {
                // Add this SSO to our list of in-process local removals so
                // this.nodeRemoved() will ignore the removal
-               //synchronized (beingLocallyRemoved)
-               {
-                  beingLocallyRemoved.add(ssoId);
-               }
                removing = true;
-               // No sessions left; remove node
+               beingLocallyRemoved.set(ssoId);
                removeFromTreeCache(getSingleSignOnFqn(ssoId));
             }
             else
@@ -566,10 +552,7 @@
          {
             if (removing)
             {
-               //synchronized (beingLocallyRemoved)
-               {
-                  beingLocallyRemoved.remove(ssoId);
-               }
+               beingLocallyRemoved.set(null);
             }
          }
          finally
@@ -668,20 +651,17 @@
    public void nodeRemoved(Fqn fqn)
    {
       String ssoId = getIdFromFqn(fqn);
+      
+      if (ssoId == null)
+         return;
 
       // Ignore messages generated by our own activity
-      //synchronized(beingLocallyRemoved)
+      if (ssoId.equals(beingLocallyRemoved.get()))
       {
-         if (beingLocallyRemoved.contains(ssoId))
-         {
-            return;
-         }
+         return;
       }
       
-      //synchronized (beingRemotelyRemoved)
-      {
-         beingRemotelyRemoved.add(ssoId);
-      }
+      beingRemotelyRemoved.set(ssoId);
 
       try
       {
@@ -694,10 +674,7 @@
       }
       finally
       {
-         //synchronized(beingRemotelyRemoved)
-         {
-            beingRemotelyRemoved.remove(ssoId);
-         }
+         beingRemotelyRemoved.set(null);
       }
 
    }
@@ -726,15 +703,12 @@
          return;
       }
 
-      String ssoId = getIdFromFqn(fqn);
+      String ssoId = getIdFromFqn(fqn); // won't be null or above check fails
 
       // Ignore invocations that come as a result of our additions
-      //synchronized(beingLocallyAdded)
+      if (ssoId.equals(beingLocallyAdded.get()))
       {
-         if (beingLocallyAdded.contains(ssoId))
-         {
-            return;
-         }
+         return;
       }
 
       SingleSignOnEntry sso = ssoValve.localLookup(ssoId);
@@ -1084,12 +1058,11 @@
       String password)
    {
       SSOCredentials data = new SSOCredentials(authType, username, password);
+      
       // Add this SSO to our list of in-process local adds so
       // this.nodeModified() will ignore the addition
-      //synchronized (beingLocallyAdded)
-      {
-         beingLocallyAdded.add(ssoId);
-      }
+      beingLocallyAdded.set(ssoId);
+      
       //UserTransaction tx = null;
       try
       {
@@ -1115,10 +1088,7 @@
       }
       finally
       {
-         //synchronized (beingLocallyAdded)
-         {
-            beingLocallyAdded.remove(ssoId);
-         }
+         beingLocallyAdded.set(null);
       }
    }