clebertsuconic commented on code in PR #4972: URL: https://github.com/apache/activemq-artemis/pull/4972#discussion_r1644812334
########## artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java: ########## @@ -100,6 +104,11 @@ public boolean addBinding(final Binding binding) throws Exception { if (nameMap.putIfAbsent(binding.getUniqueName(), bindingAddressPair) != null) { throw ActiveMQMessageBundle.BUNDLE.bindingAlreadyExists(binding); } + directBindingMap.compute(binding.getAddress(), (key, value) -> { + Collection<Binding> bindingList = value == null ? new ArrayList<>() : value; + bindingList.add(binding); Review Comment: Maybe I'm being naive... but as far as I remember.. the ArrayList itself is not supposed to be used from multiple threads. The compute will do a replay, but you could still call the compute from multiple threads. I feel like you should have: ``` synchronized(bindingList) { bindingsList.add(binding); } ``` Or another data structure to be used here. similarly on the remove the queue, it should use a synchronized or use a concurrent equivalent. -- 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 --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact