CODEWITHSUNDEEP
javasonarqubesonarlint
Naveen Kumar
Naveen Kumar

Reputation: 422

Sonarlint is giving error on volatile object reference when using double locking for Singleton class in updated 4.1 version

I have a already implemented Singleton class earlier which was using double locking mechanism for Singleton instance but we got an SonarLint error of Double-checked locking should not be used (squid:S2168) on double locking code.

public class Singleton {

private static Singleton singleton;

private Singleton() {
}

public static Singleton getInstance() {
    if (singleton == null) {
        synchronized (Singleton.class) {
            if (singleton == null) {
                singleton = new Singleton();
            }
        }
    }
    return singleton;
}

}

As a fix of this issue, I thought of putting volatile keyword before Singleton object reference like below.

private static volatile Singleton singleton;

But after making this field as volatile SonarLint is giving error Non-primitive fields should not be "volatile" (squid:S3077)

Does this mean now it is not good practice to make object reference as volatile as most of the example of singleton available are like mentioned code example?

Upvotes: 3

Views: 2050

Answers (1)

davidxxx
davidxxx

Reputation: 131446

About your try, Sonar gives indeed the volatile field as workaround but it looks like that according to new issue when you use it, it contradicts itself.... Not obvious : )

but we got an SonarLint error of Double-checked locking should not be used (squid:S2168) on double locking code.

I would remove the double checked locking that is really error prone and verbose.
Eager initialization is thread safe and is in most of case fine : 

public class Singleton {

    private static final Singleton singleton = new Singleton();

    private Singleton() {
    }

    public static Singleton getInstance() {   
        return singleton;
    }
}

The lazy way with the holder class is an alternative too (while I generally avoid because the laziness is often not a requirement) : 

public class Singleton {

    private static class SingletonHolder{
        static final Singleton singleton = new Singleton();
    }

    private Singleton() {
    }

    public static Singleton getInstance() {   
        return SingletonHolder.singleton;
    }
}

Upvotes: 2

Related Questions