clebertsuconic commented on code in PR #4972:
URL: https://github.com/apache/activemq-artemis/pull/4972#discussion_r1638460217
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java:
##########
@@ -100,6 +102,8 @@ public boolean addBinding(final Binding binding) throws
Exception {
if (nameMap.putIfAbsent(binding.getUniqueName(), bindingAddressPair) !=
null) {
throw ActiveMQMessageBundle.BUNDLE.bindingAlreadyExists(binding);
}
+ directBindingMap.computeIfAbsent(binding.getAddress(), (unused) -> new
ArrayList<>())
+ .add(binding);
Review Comment:
you are using ConcurrentHashMap on the directBindingsMap, however you're
using a non Concurrent list on the value (new ArrayList)
I'm wondering if we shouldn't use something more protective in multi-thread
here.
I understand it's unlikely the broker to remove or add a queue on the same
address from multiple threads.. but it's not impossible. (say an application
with a MultiCast address and applications creating and deleting subscriptions
from multiple places).
it would be a rare race, but difficult to track if it ever happened.
Shouldn't we use something more protective here.. perhaps CopyOnWrite or
anything else?
--
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
