Is there any need to add volatile keyword to guarantee thread-safe singleton class in java?

Question

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;
    }
}

Lazy initialized Singletons are a waste; Singletons in general are a bad idea. Google has written software to identify them for removal from their code. Why are you using them? — duffymo, Apr 07 '16 at 09:44

score 3 · Answer 1 · answered Apr 07 '16 at 09:55

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.

private constructor and final class is not really necessary. private constructor yes, or someone can just instantiate. But if you have a private constructor, the class is implicitly final anyway. But marking it as final makes it clearly stated of course. :-) — Vampire, Apr 07 '16 at 10:01

Dimitar Dimitrov · Answer 2 · 2016-04-07T12:48:04.223

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() { ... }
}

score 1 · Answer 3 · answered Apr 07 '16 at 09:53

1

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

answered Apr 07 '16 at 09:53

Vampire

35,631
4
76
102

2

I wouldn't direct anyone to DCL in 2016. The current standard is the [enum singleton](http://keaplogik.blogspot.fi/2013/12/the-java-enum-singleton-pattern.html). – Kayaman Apr 07 '16 at 10:19
I didn't he just asked whether his version is thread safe and this was the answer. – Vampire Apr 07 '16 at 10:23
And btw. the enum singleton pattern is nice, but you cannot do lazy initialization if you need it, can you? – Vampire Apr 07 '16 at 10:26
Well, as duffymo quite correctly mentioned, lazy initialization is pretty much a pipe dream that people think they need (even though they can't give a reason). However the enum singleton does it "natively", with the instance class loaded at first use (just like the holder pattern). – Kayaman Apr 07 '16 at 10:30

score 0 · Answer 4 · answered Apr 07 '16 at 09:49

0

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.

answered Apr 07 '16 at 09:49

KayV

12,987
11
98
148

score 0 · Answer 5 · answered Apr 07 '16 at 11:13

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.

Is there any need to add volatile keyword to guarantee thread-safe singleton class in java?

5 Answers5

Linked