[
https://issues.apache.org/jira/browse/ARTEMIS-2017?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16574023#comment-16574023
]
ASF GitHub Bot commented on ARTEMIS-2017:
-----------------------------------------
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)
> SelectorParser cache not thread-safe
> ------------------------------------
>
> Key: ARTEMIS-2017
> URL: https://issues.apache.org/jira/browse/ARTEMIS-2017
> Project: ActiveMQ Artemis
> Issue Type: Bug
> Affects Versions: 2.6.2
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Fix For: 2.7.0, 2.6.3
>
>
> Concurrent usage of {{SelectorParser.parse()}} can cause the {{cache}} to
> grow past its hard-coded size of 100. Synchronizing {{put}} operations
> eliminates this problem.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
