QPID-3474: Maintain connectionid/username Map from only JMXManagedObj…

…ectRegistry (Improved separation of concerns).

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

qpid/java/broker/src/main/java/org/apache/qpid/server/management/JMXManagedObjectRegistry.java

@@ -44,6 +44,7 @@
 import javax.management.JMException;
 import javax.management.MBeanServer;
 import javax.management.MBeanServerFactory;
+import javax.management.Notification;
 import javax.management.NotificationFilterSupport;
 import javax.management.NotificationListener;
 import javax.management.ObjectName;
@@ -235,9 +236,8 @@ public void start() throws IOException, ConfigurationException
             /**
              * Override makeClient so we can cache the username of the client in a Map keyed by connectionId.
              * ConnectionId is guaranteed to be unique per client connection, according to the JMS spec.
-             *
-             * MBeanInvocationHandlerImpl#handleNotification is responsible for removing the map entry on receipt
-             * of CLOSE or FAIL notifications.
+             * An instance of NotificationListener (mapCleanupListener) will be responsible for removing these Map
+             * entries.
              *
              * @see javax.management.remote.rmi.RMIJRMPServerImpl#makeClient(java.lang.String, javax.security.auth.Subject)
              */
@@ -250,6 +250,19 @@ protected RMIConnection makeClient(String connectionId, Subject subject) throws
                 return makeClient;
             }
         };
+
+        // Create a Listener responsible for removing the map entries add by the #makeClient entry above.
+        final NotificationListener mapCleanupListener = new NotificationListener()
+        {
+
+            @Override
+            public void handleNotification(Notification notification, Object handback)
+            {
+                final String connectionId = ((JMXConnectionNotification) notification).getConnectionId();
+                connectionIdUsernameMap.remove(connectionId);
+            }
+        };
+
         String localHost;
         try
         {
@@ -326,13 +339,21 @@ public JMXServiceURL getAddress()
         // which is the MBeanInvocationHandlerImpl and so also a NotificationListener.
         final NotificationListener invocationHandler = (NotificationListener) Proxy.getInvocationHandler(mbsf);
 
-        // Install a notification listener on OPENED, CLOSED, and FAIL,
+        // Install a notification listener on OPENED, CLOSED, and FAILED,
         // passing the map of connection-ids to usernames as hand-back data.
-        NotificationFilterSupport filter = new NotificationFilterSupport();
-        filter.enableType(JMXConnectionNotification.OPENED);
-        filter.enableType(JMXConnectionNotification.CLOSED);
-        filter.enableType(JMXConnectionNotification.FAILED);
-        _cs.addNotificationListener(invocationHandler, filter, connectionIdUsernameMap);
+        final NotificationFilterSupport invocationHandlerFilter = new NotificationFilterSupport();
+        invocationHandlerFilter.enableType(JMXConnectionNotification.OPENED);
+        invocationHandlerFilter.enableType(JMXConnectionNotification.CLOSED);
+        invocationHandlerFilter.enableType(JMXConnectionNotification.FAILED);
+        _cs.addNotificationListener(invocationHandler, invocationHandlerFilter, connectionIdUsernameMap);
+
+        // Install a second notification listener on CLOSED AND FAILED only to remove the entry from the
+        // Map.  Here we rely on the fact that JMX will call the listeners in the order in which they are
+        // installed.
+        final NotificationFilterSupport mapCleanupHandlerFilter = new NotificationFilterSupport();
+        mapCleanupHandlerFilter.enableType(JMXConnectionNotification.CLOSED);
+        mapCleanupHandlerFilter.enableType(JMXConnectionNotification.FAILED);
+        _cs.addNotificationListener(mapCleanupListener, mapCleanupHandlerFilter, null);
 
         _cs.start();
 

qpid/java/broker/src/main/java/org/apache/qpid/server/management/MBeanInvocationHandlerImpl.java

@@ -338,11 +338,10 @@ public void handleNotification(final Notification notification, final Object han
 
         // Normally JMXManagedObjectRegistry provides a Map as handback data containing a map
         // between connection id and username.
-        Map<String, String> connectionIdUsernameMap = null;
         String user = null;
         if (handback != null && handback instanceof Map)
         {
-            connectionIdUsernameMap = (Map<String, String>) handback;
+            final Map<String, String> connectionIdUsernameMap = (Map<String, String>) handback;
             user = connectionIdUsernameMap.get(connectionId);
         }
 
@@ -361,11 +360,6 @@ else if (JMXConnectionNotification.CLOSED.equals(type) ||
                  JMXConnectionNotification.FAILED.equals(type))
         {
             _logActor.message(ManagementConsoleMessages.CLOSE(user));
-            // We are responsible for removing the entry from the map
-            if (connectionIdUsernameMap != null)
-            {
-                connectionIdUsernameMap.remove(connectionId);
-            }
         }
     }
 }