Is there any need to add volatile keyword to guara

问题:

According to this post, the thread-safe singleton class should look as below. But I'm wondering whether there's a need to add volatile keyword to static CrunchifySingleton instance variable. Since if the instance is created and stored in CPU cache, at which time it is not written back to main memory, meanwhile, another thread invoke on getInstance() method. Will it incur an inconsistency problem?

public class CrunchifySingleton {

    private static CrunchifySingleton instance = null;

    protected CrunchifySingleton() {
    }

    // Lazy Initialization
    public static CrunchifySingleton getInstance() {
        if (instance == null) {
            synchronized (CrunchifySingleton.class) {
                if (instance == null) {
                    instance = new CrunchifySingleton();
                }
            }
        }
        return instance;
    }
}

回答1:

I echo @duffymo's comment above: lazy singletons are nowhere near as useful as they initially appear.

However, if you absolutely must use a lazily-instantiated singleton, the lazy holder idiom is much a easier way to achieve thread safety:

public final class CrunchifySingleton {
  private static class Holder {
    private static final CrunchifySingleton INSTANCE = new CrunchifySingleton();
  }

  private CrunchifySingleton() {}

  static CrunchifySingleton getInstance() { return Holder.INSTANCE; }
}

Also, note that to be truly singleton, the class needs to prohibit both instantiation and subclassing - the constructor needs to be private, and the class needs to be final, respectively.

回答2:

Yep, if your Singleton instance is not volatile or even if it is volatile but you are using sufficiently old JVM, there's no ordering guarantees for the operations in which the line

instance = new CrunchifySingleton();

decomposes with regard to the volatile store.

The compiler can then reorder these operations so that your instance is not null (because memory has been allocated), but is still uninitialized (because its constructor still hasn't been executed).

If you want to read more about the hidden problems around Double-Checked Locking, specifically in Java, see The "Double-Checked Locking is Broken" Declaration.

The lazy holder idiom is a nice pattern that generalizes well for general static field lazy loading, but if you need a safe and simple Singleton pattern, I'd recommend what Josh Bloch (from Effective Java fame) recommends - the Java Enum Singleton:

public enum Elvis {
    INSTANCE;

    public void leaveTheBuilding() { ... }
}

回答3:

The code how you quoted it is broken in Java. Yes, you need volatile and at least Java 5 to make the double-checked idiom thread safe. And you should also add a local variable in your lazy initialization to improve performance. Read more about it here: https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java

回答4:

Yes, making volatile will gaurantee that every time any thread is trying to access your critical section of code, the thread reads the data from memory itself and not from hread cache.

回答5:

You need volatile in this case but a better option is to use either an enum if it is stateless

enum Singleton {
     INSTANCE;
}

however stateful singletons should be avoid every possible. I suggest you try creating an instance which you pass via dependency injection.