You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2020/10/12 10:37:25 UTC

[GitHub] [qpid-broker-j] dakirily opened a new pull request #64: QPID-8477: Broker is not closing connections denied with ACL-1002

dakirily opened a new pull request #64:
URL: https://github.com/apache/qpid-broker-j/pull/64


   Added closing sender when closing AMQP connection


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-broker-j] alex-rufous commented on a change in pull request #64: QPID-8477: Broker is not closing connections denied with ACL-1002

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-broker-j] asfgit closed pull request #64: QPID-8477: Broker is not closing connections denied with ACL-1002

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #64:
URL: https://github.com/apache/qpid-broker-j/pull/64


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org