QPID-2196: updated application and server registries to better keep track of the connection status, allowing prevention of JMXConnector usage (eg refreshing the view) after the connection has already been closed remotely.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@834848 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 89 insertions, 10 deletions

diff --git a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ApplicationRegistry.java b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ApplicationRegistry.java
index 1f288a6390..0f5f5267fd 100644
--- a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ApplicationRegistry.java
+++ b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ApplicationRegistry.java
@@ -21,9 +21,8 @@
 package org.apache.qpid.management.ui;
 
 import java.io.File;
-import java.io.IOException;
-import java.util.HashMap;
 import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.eclipse.jface.resource.FontRegistry;
@@ -91,9 +90,8 @@ public abstract class ApplicationRegistry
     
     /*
      * This maps all the managed servers to the respective server registry.
-     * Server can be JMX MBeanServer or a C++ server
      */
-    private static HashMap<ManagedServer, ServerRegistry> _serverRegistryMap = new HashMap<ManagedServer, ServerRegistry>();
+    private static ConcurrentHashMap<ManagedServer, ServerRegistry> _serverRegistryMap = new ConcurrentHashMap<ManagedServer, ServerRegistry>();
     
     // This map gets updated when a server connection closes.
     private static List<ManagedServer> _closedServerList = new CopyOnWriteArrayList<ManagedServer>();    
@@ -131,7 +129,19 @@ public abstract class ApplicationRegistry
     
     public static boolean isServerConnected(ManagedServer server)
     {
-        return _serverRegistryMap.containsKey(server);
+        if(server == null)
+        {
+            //checking for null is not permitted in a CHM
+            return false;
+        }
+        
+        ServerRegistry reg = _serverRegistryMap.get(server);
+        if(reg !=null)
+        {
+            return !reg.isServerConnectionClosed();
+        }
+
+        return false;
     }
     
     // remove the server from the registry
@@ -141,6 +151,27 @@ public abstract class ApplicationRegistry
         removeServer(server);
     }
     
+    // remove the server from the registry
+    public static void serverConnectionClosedRemotely(ManagedServer server)
+    {
+        ServerRegistry reg = _serverRegistryMap.get(server);
+        if(reg !=null)
+        {
+            synchronized(server)
+            {
+                if(reg.isServerConnectionClosed())
+                {
+                    //the connection closure was already processed
+                    return;
+                }
+
+                reg.serverConnectionClosed();
+            }
+        }
+        
+        serverConnectionClosed(server);
+    }
+    
     /*
      * Returns the lis of closed servers. The Thread in GUI, which keeps checking for closed connection
      * will check this and will remove the server links from the GUI.
diff --git a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ServerRegistry.java b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ServerRegistry.java
index 6c0b51e584..027a555360 100644
--- a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ServerRegistry.java
+++ b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/ServerRegistry.java
@@ -25,6 +25,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.qpid.management.ui.jmx.ClientListener;
 import org.apache.qpid.management.ui.model.ManagedAttributeModel;
@@ -49,6 +50,8 @@ public abstract class ServerRegistry
     // map of all virtual host manager mbeans
     private ConcurrentMap<String,ManagedBean> _vhostManagers = new ConcurrentHashMap<String,ManagedBean>();
     
+    private AtomicBoolean _serverConnectionClosed = new AtomicBoolean(false);
+    
     public ServerRegistry()
     {
         
@@ -59,6 +62,16 @@ public abstract class ServerRegistry
         _managedServer = server;
     }
     
+    public void serverConnectionClosed()
+    {
+        _serverConnectionClosed.set(true);
+    }
+    
+    public boolean isServerConnectionClosed()
+    {
+        return _serverConnectionClosed.get();
+    }
+    
     public void setManagementApiVersion(ApiVersion mgmtApiVersion)
     {
         _managementApiVersion = mgmtApiVersion;  
diff --git a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/jmx/ClientListener.java b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/jmx/ClientListener.java
index d88e0f38bb..694f7ba86b 100644
--- a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/jmx/ClientListener.java
+++ b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/jmx/ClientListener.java
@@ -58,12 +58,12 @@ public class ClientListener implements NotificationListener
         }
         else if (JMXConnectionNotification.FAILED.equals(type))
         {
-            ApplicationRegistry.serverConnectionClosed(server);
+            ApplicationRegistry.serverConnectionClosedRemotely(server);
             MBeanUtility.printOutput("JMX Connection to " + server.getName() + " failed.");
         }
         else if (JMXConnectionNotification.CLOSED.equals(type))
         {
-            ApplicationRegistry.serverConnectionClosed(server);
+            ApplicationRegistry.serverConnectionClosedRemotely(server);
             MBeanUtility.printOutput("JMX Connection to " + server.getName() + " was closed.");
         }
     }
diff --git a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/jmx/JMXServerRegistry.java b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/jmx/JMXServerRegistry.java
index ca07e6acf4..717f781334 100644
--- a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/jmx/JMXServerRegistry.java
+++ b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/jmx/JMXServerRegistry.java
@@ -118,6 +118,12 @@ public class JMXServerRegistry extends ServerRegistry
      */
     public void closeServerConnection() throws IOException
     {
+        if(isServerConnectionClosed())
+        {
+            //connection was already closed
+            return;
+        }
+        
         try
         {
             //remove the listener from the JMXConnector
@@ -165,6 +171,8 @@ public class JMXServerRegistry extends ServerRegistry
         {
             _jmxc.close();
         }
+        
+        serverConnectionClosed();
     }
     
     public ManagedBean getManagedObject(String uniqueName)
diff --git a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/views/MBeanView.java b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/views/MBeanView.java
index ffb601fdca..9763c799d7 100644
--- a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/views/MBeanView.java
+++ b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/views/MBeanView.java
@@ -110,6 +110,11 @@ public class MBeanView extends ViewPart
             
             setServer();
             
+            if(!ApplicationRegistry.isServerConnected(_server))
+            {
+                return;
+            }
+            
             if (MBEAN.equals(_selectedNode.getType()))
             {
                 _mbean = (ManagedBean)_selectedNode.getManagedObject();
@@ -488,6 +493,11 @@ public class MBeanView extends ViewPart
     
     public void refresh()
     {
+        if(!ApplicationRegistry.isServerConnected(_server))
+        {
+            return;
+        }
+        
         if (_tabFolder != null && !_tabFolder.isDisposed())
         {
             if(_tabFolder.getVisible())
diff --git a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/views/NavigationView.java b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/views/NavigationView.java
index 056d365f8e..1ca6c2b71d 100644
--- a/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/views/NavigationView.java
+++ b/java/management/eclipse-plugin/src/main/java/org/apache/qpid/management/ui/views/NavigationView.java
@@ -29,6 +29,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.qpid.management.common.mbeans.ConfigurationManagement;
 import org.apache.qpid.management.common.mbeans.LoggingManagement;
@@ -95,7 +96,7 @@ public class NavigationView extends ViewPart
 
     private PreferenceStore _preferences;
     // Map of connected servers
-    private HashMap<ManagedServer, TreeObject> _managedServerMap = new HashMap<ManagedServer, TreeObject>();
+    private ConcurrentHashMap<ManagedServer, TreeObject> _managedServerMap = new ConcurrentHashMap<ManagedServer, TreeObject>();
     
     private static HashSet<String> _serverTopLevelMBeans = new HashSet<String>();
     {
@@ -719,6 +720,11 @@ public class NavigationView extends ViewPart
      */
     private void removeManagedObject(TreeObject parent)
     {
+        if(parent == null)
+        {
+           return;
+        }
+        
         List<TreeObject> list = parent.getChildren();
         for (TreeObject child : list)
         {
@@ -1263,7 +1269,7 @@ public class NavigationView extends ViewPart
 
                 try
                 {
-                    Thread.sleep(3000);
+                    Thread.sleep(2000);
                 }
                 catch (Exception ex)
                 { }
@@ -1340,7 +1346,18 @@ public class NavigationView extends ViewPart
                     {
                         for (ManagedServer server : closedServers)
                         {
-                            removeManagedObject(_managedServerMap.get(server));
+                            if(server == null)
+                            {
+                                continue;
+                            }
+                            
+                            TreeObject node = _managedServerMap.get(server);
+                            if(node ==null)
+                            {
+                                continue;
+                            }
+                            
+                            removeManagedObject(node);
                             _managedServerMap.remove(server);
                             ApplicationRegistry.removeServer(server);
                         }