CODEWITHSUNDEEP
javaconcurrencyvolatiledouble-checked-locking
nyarian
nyarian

Reputation: 4365

Volatile reads clash

Suppose we are instantiating a singleton using double-checked locking:

public static Instance getInstance() {
    if (this.instance == null) {
        synchronized(Instance.class) {
            if (this.instance == null) {
                this.instance = new Instance();
            }
        }
    }
    return this.instance;
}

The question is in the semantics of the program if instance variable would be volatile and double-checked locking would be removed.

private volatile static Instance instance;

public static Instance getInstance() {
    if (this.instance == null) {
        this.instance = new Instance();
    }
    return this.instance;
}

Will the class get instantiated only once? Or, put another way, can volatile reads clash in such a way that two threads will see null value of the reference and double instantiation will be performed?

I am aware of the happens-before relationship between volatile write and volatile read, and that volatile forbids caching (so all reads and writes will be executed in main memory, not in processor's cache), but it isn't clear in the case of concurrent volatile reads.

P.S.: the question is not in applying of the Singleton pattern (it was just an example where the problem is clear), it is just about if double-checked locking can be replaced with volatile read - volatile write without program semantics change, nothing more than that.

Upvotes: 1

Views: 100

Answers (4)

Krzysztof Cichocki
Krzysztof Cichocki

Reputation: 6414

Yes, indeed: volatile reads can clash in such way that two threads will see null value of the reference and double instantiation will be performed.

You need double brace initialization and volatile as well. That's because when instance becomes non null, you don't synchronize other threads on anything before reading it - first if just makes them go further to return the unsynchronized value (even if the initializing thread doesn't escape the synchronized block yet), this could result in a subsequent thread reading not initialized variable because of the lack of synchroniaztion. Synchronization to work properly needs to be performed by every thread accessing the data governed by it, the DCL omits the synchronization after initialization which is wrong. That's why you need additionaly volatile for the DCL to work, then the volatile will ensure that you read initialized value.

There is no such thing as processor cache separation, reads and writes are visible right away, but there are instruction rearrangements, so in favor for optimization processor can invoke some instructions later in time if it doesn't need their results right away. The whole point of synchronization and volatile is to not rearrange the order of instructions for threads accesing them. That way if something is synchronized and is stated as done in code, it is really done and other threads can access it safely. That's the whole point of happens before guarantee.

Summing this together: without proper synchronization processor can initialize the reference to the instance to be non null, but instance could be not fully initialized internally, so subsequent thread reading it could read uninitialized object and behave wrongly because of that.

Upvotes: 2

Naomi
Naomi

Reputation: 5486

Without synchronization, your code is definitely broken, because 2 threads may see that the value of instance is null, and both will perform an initialization (consider a context switch at every line, and see what happens.

Beside that, even with synchronization double checked locking (DCL) was considered broken in Java in the past because when running unsynchronized, a second thread may experience the initialization operations in a different order. You may fix your code by adding a local variable and load the volatile into it whenever you want to read it:

public static Instance getInstance() {
    Instance tmp = instance;
    if (tmp == null) {
        synchronized(Instance.class) {
            Instance tmp = instance;
            if (tmp == null) {
                instance = new Instance();
            }
        }
    }
    return instance;
}

but a safer solution would be using the ClassLoader as your synchronization mechanism, and will also allow you to stop using the slow volatile access every time you access the singleton:

public class Instance {

    private static class Lazy {
        private static Instance INSTANCE = new Instance();    
    }

    public static Instance getInstance() {
        return Lazy.INSTANCE;
    }
}

INSTANCE will be initialized only when the first thread enters getInstance()

Upvotes: 2

marstran
marstran

Reputation: 28036

Use an enum if you really need a singleton in Java. It fixes these things for you:

enum MySingleton {
    INSTANCE;

    public static MySingleton getInstance() {
        return INSTANCE;
    }
}

The JVM will initialize the instance in a thread safe manner on the first access to MySingleton. The only way to get more than one instance of it, is if you have multiple classloaders.

Upvotes: 0

user10367961
user10367961

Reputation:

Considering this code.

private volatile static Instance instance;

public static Instance getInstance() {
    if (this.instance == null) {
        this.instance = new Instance();
    }
    return this.instance;
}

From your question:

Will the class get instantiated only once? Can volatile reads clash in such way that two threads will see null value of the reference and double instantiation will be performed?

Volatile reads can't clash in such a way out of JMM guarantees. You can still end up with two instances though, if multiple threads swap after the if but before beginning to instantiate the volatile variable. 

if (this.instance == null) {
    // if threads swap out here you get multiple instances
    this.instance = new Instance();
}

In order to ensure the above scenario does not happen, you have to use double checked locking 

if (this.instance == null) {
    // threads can swap out here (after first if but before synchronized)
    synchronized(Instance.class) {
        if (this.instance == null) {
            // but only one thread will get here
            this.instance = new Instance();
        }
    }
}

Note that there are two aspects that have to be considered here.

  • Atomicity: we need to make sure that the second if and the instantiation happen in an atomic manner (this is why we need the synchronized block).
  • Visibility: we need to make sure that the reference to the instance variable does not escape in an inconsistent state (this is why we need the volatile declaration for the instance variable in order to leverage the JMM happens before guarantee).

Upvotes: 1

Related Questions