Github user michaelandrepearce commented on a diff in the pull request: https://github.com/apache/activemq-artemis/pull/2228#discussion_r208765467 --- Diff: artemis-selector/src/main/java/org/apache/activemq/artemis/selector/impl/SelectorParser.java --- @@ -80,11 +78,15 @@ public static BooleanExpression parse(String sql) throws FilterException { StrictParser parser = new StrictParser(new StringReader(actual)); e = parser.JmsSelector(); } - cache.put(sql, e); + synchronized (cache) { --- End diff -- I would have two points here then: If perf here isn't critical, why even bother then having an LRUCache? Removing it would remove any threading issues right. If it is critical: Then a more elegant option than sync blocks would look to update LRUCache to extend ConcurrentLinkedHashMap (https://github.com/ben-manes/concurrentlinkedhashmap) or even better full hog replace it entirely and migrate to using something like Caffeine (https://github.com/ben-manes/caffeine)
---