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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
