benwtrent commented on code in PR #13306: URL: https://github.com/apache/lucene/pull/13306#discussion_r1583351253
########## lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java: ########## @@ -628,7 +632,7 @@ private class LeafCache implements Accountable { LeafCache(Object key) { this.key = key; - cache = new IdentityHashMap<>(); + cache = Collections.synchronizedMap(new IdentityHashMap<>()); Review Comment: I don't understand this. Aren't all accesses to `LeafCache` protected by the LRUQueryCache read/write locks? Since `LeafCache` isn't a static class, it should have access to the enclosing class's lock. For testing safety, putIfAbsent, remove, onDocIdSetCache, and onDocIdSetEviction should all do a `assert writeLock.isHeldByCurrentThread();` -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org