[jboss-cvs] JBossAS SVN: r108868 - projects/cluster/ha-server-core/trunk/src/main/java/org/jboss/ha/core/framework/server.

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

Author: pferraro
Date: 2010-10-24 23:46:51 -0400 (Sun, 24 Oct 2010)
New Revision: 108868

Modified:
   projects/cluster/ha-server-core/trunk/src/main/java/org/jboss/ha/core/framework/server/DistributedReplicantManagerImpl.java
Log:
Drop use of latch to defend against null nodeName.  As implemented, this is causing deadlocked threads when a call to a method requiring await() arrives after stopService() is called (i.e. when the latch is reset).
This was a sledgehammer fix to an issue that was avoidable to begin with by using the node name from the partition itself, instead of a local variable.

Modified: projects/cluster/ha-server-core/trunk/src/main/java/org/jboss/ha/core/framework/server/DistributedReplicantManagerImpl.java
===================================================================
--- projects/cluster/ha-server-core/trunk/src/main/java/org/jboss/ha/core/framework/server/DistributedReplicantManagerImpl.java	2010-10-25 01:54:11 UTC (rev 108867)
+++ projects/cluster/ha-server-core/trunk/src/main/java/org/jboss/ha/core/framework/server/DistributedReplicantManagerImpl.java	2010-10-25 03:46:51 UTC (rev 108868)
@@ -34,7 +34,6 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.management.MBeanServer;
@@ -86,11 +85,6 @@
    
    final Logger log;
    
-   private String nodeName = null;
-   
-   // Works like a simple latch
-   private volatile CountDownLatch partitionNameKnown = new CountDownLatch(1);
-
    // Static --------------------------------------------------------
    
    // Constructors --------------------------------------------------
@@ -125,11 +119,7 @@
    
    public void startService() throws Exception
    {
-      this.nodeName = this.partition.getNodeName();
-      
       this.asynchHandler.start();
-
-      this.partitionNameKnown.countDown(); // partition name is now known!
       
       //log.info("mergemembers");
       //mergeMembers();
@@ -146,9 +136,6 @@
       {
          this.log.warn("Failed to stop asynchHandler", e);
       }
-      
-      // Reset the latch
-      this.partitionNameKnown = new CountDownLatch(1);
    }
 
    // NR 200505 : [JBCLUSTER-38] unbind at destroy
@@ -265,7 +252,7 @@
          if (local != null)
          {
             result.append("\t\t<Location>\n");
-            result.append("\t\t\t<Name local=\"True\">").append (this.nodeName).append ("</Name>\n");
+            result.append("\t\t\t<Name local=\"True\">").append (this.partition.getNodeName()).append ("</Name>\n");
             result.append("\t\t</Location>\n");
          }
 
@@ -311,7 +298,7 @@
          
          if (local != null)
          {
-            map.put(this.nodeName, local);
+            map.put(this.partition.getNodeName(), local);
          }
          
          result.put(category, map);
@@ -335,7 +322,7 @@
 
       if (this.log.isTraceEnabled())
       {
-         this.log.trace(this.nodeName + ": received new state, will republish local replicants");
+         this.log.trace(this.partition.getNodeName() + ": received new state, will republish local replicants");
       }
       
       new MembersPublisher().start();
@@ -379,7 +366,7 @@
       // Here we only care about deadMembers.  Purge all replicant lists of deadMembers
       // and then notify all listening nodes.
       //
-      this.log.info("I am (" + this.nodeName + ") received membershipChanged event:");
+      this.log.info("I am (" + this.partition.getNodeName() + ") received membershipChanged event:");
       this.log.info("Dead members: " + deadMembers.size() + " (" + deadMembers + ")");
       this.log.info("New Members : " + newMembers.size()  + " (" + newMembers + ")");
       this.log.info("All Members : " + allMembers.size()  + " (" + allMembers + ")");
@@ -414,10 +401,8 @@
          this.log.trace("add, key=" + key + ", value=" + replicant);
       }
       
-      this.partitionNameKnown.await(); // we don't propagate until our name is known
+      Object[] args = { key, this.partition.getNodeName(), replicant };
       
-      Object[] args = { key, this.nodeName, replicant };
-      
       this.partition.callMethodOnCluster(SERVICE_NAME, "_add", args, add_types, true);
 
       List<Serializable> replicants = null;
@@ -435,13 +420,11 @@
    @Override
    public void remove(String key) throws Exception
    {
-      this.partitionNameKnown.await(); // we don't propagate until our name is known
-      
       // optimisation: we don't make a costly network call
       // if there is nothing to remove
       if (this.localReplicants.containsKey(key))
       {
-         Object[] args = { key, this.nodeName };
+         Object[] args = { key, this.partition.getNodeName() };
          
          this.partition.callAsynchMethodOnCluster(SERVICE_NAME, "_remove", args, remove_types, true);
          
@@ -494,7 +477,7 @@
       {
          String name = node.getName();
          
-         if (local != null && this.nodeName.equals(name))
+         if (local != null && this.partition.getNodeName().equals(name))
          {
             result.add(local);
          }
@@ -565,7 +548,7 @@
       {
          String name = node.getName();
          
-         if (local && this.nodeName.equals(name))
+         if (local && this.partition.getNodeName().equals(name))
          {
             rtn.add(this.partition.getClusterNode());
          }
@@ -663,7 +646,7 @@
             }
             return false;
          }
-         else if (node.equals(this.nodeName))
+         else if (node.equals(this.partition.getNodeName()))
          {
             if (this.log.isTraceEnabled())
             {
@@ -770,10 +753,8 @@
     */
    public Object[] lookupLocalReplicants() throws Exception
    {
-      this.partitionNameKnown.await(); // we don't answer until our name is known
+      Object[] rtn = { this.partition.getNodeName(), this.localReplicants };
       
-      Object[] rtn = { this.nodeName, this.localReplicants };
-      
       if (this.log.isTraceEnabled())
       {
          this.log.trace("lookupLocalReplicants called ("+ rtn[0] + "). Return: " + this.localReplicants.size());
@@ -909,7 +890,7 @@
                   this.log.trace("publishing, key=" + key + ", value=" + replicant);
                }
 
-               Object[] args = { key, this.nodeName, replicant };
+               Object[] args = { key, this.partition.getNodeName(), replicant };
 
                this.partition.callAsynchMethodOnCluster(SERVICE_NAME, "_add", args, add_types, true);