alex-rufous commented on a change in pull request #64:
URL: https://github.com/apache/qpid-broker-j/pull/64#discussion_r505093207



##########
File path: 
broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/AMQPConnection_1_0Impl.java
##########
@@ -1197,6 +1197,9 @@ private void closeConnection(final Error error)
             default:
                 throw new ServerScopedRuntimeException("Unknown state: " + 
_connectionState);
         }
+
+        getSender().close();

Review comment:
       Closing the network connection at line 1201 looks wrong to me.  If 
connection state is OPEN, the counterpart should get a chance to respond with a 
CLOSE perforrmative after sending CLOSE by broker. I would argue that more 
correct fix for the issue would be to close the network connection only when 
state is AWAIT_OPEN as illustrated below
   
   `
   --- 
a/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/AMQPConnection_1_0Impl.java
   +++ 
b/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/AMQPConnection_1_0Impl.java
   @@ -1179,6 +1179,7 @@ public class AMQPConnection_1_0Impl extends 
AbstractAMQPConnection<AMQPConnectio
                    sendOpen(0, 0);
                    sendClose(close);
                    _connectionState = ConnectionState.CLOSED;
   +                getSender().close();
                    break;
                case OPENED:
                    sendClose(close);
   `
   
   
   
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to