dweiss commented on code in PR #13087: URL: https://github.com/apache/lucene/pull/13087#discussion_r1486863725
########## lucene/replicator/src/test/org/apache/lucene/replicator/nrt/TestSimpleServer.java: ########## @@ -61,7 +61,7 @@ @SuppressForbidden(reason = "We need Unsafe to actually crush :-)") public class TestSimpleServer extends LuceneTestCase { - static final Set<Thread> clientThreads = Collections.synchronizedSet(new HashSet<>()); Review Comment: Seems fine even here as it only affects one test. ########## lucene/core/src/java/org/apache/lucene/util/VirtualMethod.java: ########## @@ -74,14 +73,13 @@ */ public final class VirtualMethod<C> { - private static final Set<Method> singletonSet = Review Comment: Same here, perhaps (or merge with the issue above)? ########## lucene/core/src/java/org/apache/lucene/store/NativeFSLockFactory.java: ########## @@ -65,7 +64,7 @@ public final class NativeFSLockFactory extends FSLockFactory { /** Singleton instance */ public static final NativeFSLockFactory INSTANCE = new NativeFSLockFactory(); - private static final Set<String> LOCK_HELD = Collections.synchronizedSet(new HashSet<String>()); Review Comment: There are subtle differences between these two things (synchronized set and a concurrent set). I would leave this change to a separate issue so that it can be assessed separately (I think it's a good idea, but we need to track down all the uses and make sure nothing will break). -- 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