javanna commented on code in PR #16755:
URL: https://github.com/apache/lucene/pull/16755#discussion_r4155185648


##########
lucene/analysis/morfologik/src/java/org/apache/lucene/analysis/uk/UkrainianMorfologikAnalyzer.java:
##########
@@ -70,6 +70,9 @@ public final class UkrainianMorfologikAnalyzer extends 
StopwordAnalyzerBase {
   private static DefaultResources getDefaultResources() {
     if (defaultResources == null) {
       synchronized (DefaultResources.class) {
+        if (defaultResources != null) {
+          return defaultResources;

Review Comment:
   I understand the problem and the fix proposed looks ok. Though I think this 
use of synchronized + volatile is trappy and that's what caused this bug in the 
first place. It then makes me want to add tricky tests to cover the race as 
well. 
   
   How about we refactor this and do as we do in other places like 
`PolishAnalyzer`?  We could use a default holder instance, initialized in a 
static block, no volatile, no synchronized, and no private non final static 
member that requires a suppress annotation. Would that work?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to