gtully commented on code in PR #4700:
URL: https://github.com/apache/activemq-artemis/pull/4700#discussion_r1413670268


##########
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireProtocolManager.java:
##########
@@ -244,18 +244,15 @@ public void nodeDown(long eventUID, String nodeID) {
       }
    }
 
-   public void removeConnection(ConnectionInfo info, Throwable error) throws 
InvalidClientIDException {
-      String clientId = info.getClientId();
-      if (clientId != null) {
-         AMQConnectionContext context = this.clientIdSet.remove(clientId);
-         if (context != null) {
-            //connection is still there and need to close
-            context.getConnection().disconnect(error != null);
-            this.connections.remove(context.getConnection());
+   public void removeConnection(String clientID, OpenWireConnection 
connection) {
+      synchronized (clientIdSet) {

Review Comment:
   without any reconnect on retained state, we can safely drop the sync and 
live with concurrent map, just check for the race to put, that is now the only 
point of contention as we always create new.



-- 
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.

To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to