QPID-3756: Reorder Java Broker's connection close process so that ConnectionCloseOk is sent only after first updating its internal state.

This change means that the connection mbean will be deregistered from JMX before sending ConnectionCloseOk, thus removing the
possibility of the race condition.  The client side process is unaffected by this change.

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

3 files changed, 17 insertions, 7 deletions

diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
index 0bf65d388b..02c4ffa012 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
@@ -20,6 +20,8 @@
  */
 package org.apache.qpid.server.transport;
 
+import static org.apache.qpid.transport.Connection.State.CLOSE_RCVD;
+
 import org.apache.qpid.common.ServerPropertyNames;
 import org.apache.qpid.properties.ConnectionStartProperties;
 import org.apache.qpid.protocol.ProtocolEngine;
@@ -132,17 +134,20 @@ public class ServerConnectionDelegate extends ServerDelegate
         }
     }
 
+    @Override
     public void connectionClose(Connection conn, ConnectionClose close)
     {
+        final ServerConnection sconn = (ServerConnection) conn;
         try
         {
-            ((ServerConnection) conn).logClosed();
+            sconn.logClosed();
         }
         finally
         {
-            super.connectionClose(conn, close);
+            sconn.closeCode(close);
+            sconn.setState(CLOSE_RCVD);
+            sendConnectionCloseOkAndCloseSender(conn);
         }
-        
     }
 
     public void connectionOpen(Connection conn, ConnectionOpen open)
@@ -196,7 +201,7 @@ public class ServerConnectionDelegate extends ServerDelegate
         {
             LOGGER.error("Connection '" + sconn.getConnectionId() + "' being severed, " +
                     "client connectionTuneOk returned a channelMax (" + okChannelMax +
-                    ") above the servers offered limit (" + getChannelMax() +")");
+                    ") above the server's offered limit (" + getChannelMax() +")");
 
             //Due to the error we must forcefully close the connection without negotiation
             sconn.getSender().close();
diff --git a/java/common/src/main/java/org/apache/qpid/transport/Connection.java b/java/common/src/main/java/org/apache/qpid/transport/Connection.java
index fbf53996a8..b0f1a1bad8 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/Connection.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/Connection.java
@@ -512,7 +512,7 @@ public class Connection extends ConnectionInvoker
         exception(new ConnectionException(t));
     }
 
-    void closeCode(ConnectionClose close)
+    public void closeCode(ConnectionClose close)
     {
         synchronized (lock)
         {
diff --git a/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java b/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java
index c70cab85c9..fdd35d49ef 100644
--- a/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java
+++ b/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java
@@ -71,12 +71,17 @@ public abstract class ConnectionDelegate
 
     @Override public void connectionClose(Connection conn, ConnectionClose close)
     {
-        conn.connectionCloseOk();
-        conn.getSender().close();
+        sendConnectionCloseOkAndCloseSender(conn);
         conn.closeCode(close);
         conn.setState(CLOSE_RCVD);
     }
 
+    protected void sendConnectionCloseOkAndCloseSender(Connection conn)
+    {
+        conn.connectionCloseOk();
+        conn.getSender().close();
+    }
+
     @Override public void connectionCloseOk(Connection conn, ConnectionCloseOk ok)
     {
         conn.getSender().close();