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