You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kw...@apache.org on 2012/02/06 15:41:54 UTC

svn commit: r1241024 - in /qpid/trunk/qpid/java: broker/src/main/java/org/apache/qpid/server/transport/ common/src/main/java/org/apache/qpid/transport/

Author: kwall
Date: Mon Feb  6 14:41:53 2012
New Revision: 1241024

URL: http://svn.apache.org/viewvc?rev=1241024&view=rev
Log:
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.

Modified:
    qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
    qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java
    qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java

Modified: qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java?rev=1241024&r1=1241023&r2=1241024&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java (original)
+++ qpid/trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java Mon Feb  6 14:41:53 2012
@@ -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 ex
         }
     }
 
+    @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 ex
         {
             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();

Modified: qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java?rev=1241024&r1=1241023&r2=1241024&view=diff
==============================================================================
--- qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java (original)
+++ qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java Mon Feb  6 14:41:53 2012
@@ -512,7 +512,7 @@ public class Connection extends Connecti
         exception(new ConnectionException(t));
     }
 
-    void closeCode(ConnectionClose close)
+    public void closeCode(ConnectionClose close)
     {
         synchronized (lock)
         {

Modified: qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java?rev=1241024&r1=1241023&r2=1241024&view=diff
==============================================================================
--- qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java (original)
+++ qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java Mon Feb  6 14:41:53 2012
@@ -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();



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org