QPID-3756: ManagedConnectionMBeanTest.testChannels occasionally fails on 0-10 spawned profiles

This problem is caused by the connection mbean being deregistered too late (as the TCP/IP socket)
is closed, thus creating a race between the client using JMX to observe connection state, and the
unregistering of the bean.  This patch changes the 0-10 code path to unregister the bean on receipt
of ConnectionClose and before ConnectionCloseOkay is sent to the client.

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

2 files changed, 38 insertions, 20 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnection.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnection.java
index ab07ed20f6..04cdbf2b25 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnection.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnection.java
@@ -62,6 +62,8 @@ import org.apache.qpid.transport.ExecutionException;
 import org.apache.qpid.transport.Method;
 import org.apache.qpid.transport.ProtocolEvent;
 import org.apache.qpid.transport.Session;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class ServerConnection extends Connection implements Managable, AMQConnectionModel, LogSubject, AuthorizationHolder
 {
@@ -123,6 +125,7 @@ public class ServerConnection extends Connection implements Managable, AMQConnec
             {
                 _virtualHost.getConnectionRegistry().deregisterConnection(this);
             }
+            unregisterConnectionMbean();
         }
 
         if (state == State.CLOSED)
@@ -161,15 +164,7 @@ public class ServerConnection extends Connection implements Managable, AMQConnec
 
         initialiseStatistics();
 
-        try
-        {
-            _mBean = new ServerConnectionMBean(this);
-            _mBean.register();
-        }
-        catch (JMException jme)
-        {
-            log.error("Unable to create mBean for ServerConnection",jme);
-        }
+        registerConnectionMbean();
     }
 
     public void setConnectionConfig(final ConnectionConfig config)
@@ -285,11 +280,7 @@ public class ServerConnection extends Connection implements Managable, AMQConnec
     public void close(AMQConstant cause, String message) throws AMQException
     {
         closeSubscriptions();
-        if (_mBean != null)
-        {
-            _mBean.unregister();
-            _mBean = null;
-        }
+        unregisterConnectionMbean();
         ConnectionCloseCode replyCode = ConnectionCloseCode.NORMAL;
         try
         {
@@ -433,11 +424,6 @@ public class ServerConnection extends Connection implements Managable, AMQConnec
     public void closed()
     {
         closeSubscriptions();
-        if (_mBean != null)
-        {
-            _mBean.unregister();
-            _mBean = null;
-        }
         super.closed();
     }
 
@@ -483,4 +469,30 @@ public class ServerConnection extends Connection implements Managable, AMQConnec
             _mBean.notifyClients("Channel count (" + channelsCount + ") has reached the threshold value");
         }
     }
+
+    private void registerConnectionMbean()
+    {
+        try
+        {
+            _mBean = new ServerConnectionMBean(this);
+            _mBean.register();
+        }
+        catch (JMException jme)
+        {
+            log.error("Unable to register mBean for ServerConnection", jme);
+        }
+    }
+
+    private void unregisterConnectionMbean()
+    {
+        if (_mBean != null)
+        {
+            if (log.isDebugEnabled())
+            {
+                log.debug("Unregistering mBean for ServerConnection" + _mBean);
+            }
+            _mBean.unregister();
+            _mBean = null;
+        }
+    }
 }
diff --git a/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagedConnectionMBeanTest.java b/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagedConnectionMBeanTest.java
index 5f758061d5..7f578a6389 100644
--- a/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagedConnectionMBeanTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/management/jmx/ManagedConnectionMBeanTest.java
@@ -22,6 +22,7 @@ import java.io.IOException;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
+
 import javax.jms.Connection;
 import javax.jms.Destination;
 import javax.jms.Message;
@@ -37,9 +38,13 @@ import org.apache.qpid.client.AMQSession;
 import org.apache.qpid.management.common.mbeans.ManagedConnection;
 import org.apache.qpid.test.utils.JMXTestUtils;
 import org.apache.qpid.test.utils.QpidBrokerTestCase;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class ManagedConnectionMBeanTest extends QpidBrokerTestCase
 {
+    private static final Logger LOGGER = LoggerFactory.getLogger(ManagedConnectionMBeanTest.class);
+
     /**
      * JMX helper.
      */
@@ -120,9 +125,10 @@ public class ManagedConnectionMBeanTest extends QpidBrokerTestCase
 
         _connection.close();
 
+        LOGGER.debug("Querying JMX for number of open connections");
         connections = _jmxUtils.getManagedConnections("test");
         assertNotNull("Connection MBean is not found", connections);
-        assertEquals("Unexpected number of connection mbeans", 0, connections.size());
+        assertEquals("Unexpected number of connection mbeans after connection closed", 0, connections.size());
     }
 
     public void testCommit() throws Exception