Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What is the memory visibility of variables accessed in static singletons in Java?

I've seen this type of code a lot in projects, where the application wants a global data holder, so they use a static singleton that any thread can access.

public class GlobalData {

    // Data-related code. This could be anything; I've used a simple String.
    //
    private String someData;
    public String getData() { return someData; }
    public void setData(String data) { someData = data; }

    // Singleton code
    //
    private static GlobalData INSTANCE;
    private GlobalData() {}
    public synchronized GlobalData getInstance() { 
       if (INSTANCE == null) INSTANCE = new GlobalData();
       return INSTANCE; 
    }
}

I hope it's easy to see what's going on. One can call GlobalData.getInstance().getData() at any time on any thread. If two threads call setData() with different values, even if you can't guarantee which one "wins", I'm not worried about that.

But thread-safety isn't my concern here. What I'm worried about is memory visibility. Whenever there's a memory barrier in Java, the cached memory is synched between the corresponding threads. A memory barrier happens when passing through synchronizations, accessing volatile variables, etc.

Imagine the following scenario happening in chronological order:

// Thread 1
GlobalData d = GlobalData.getInstance();
d.setData("one");

// Thread 2
GlobalData d = GlobalData.getInstance();
d.setData("two");

// Thread 1
String value = d.getData();

Isn't it possible that the last value of value in thread 1 can still be "one"? The reason being, thread 2 never called any synchronized methods after calling d.setData("two") so there was never a memory barrier? Note that the memory-barrier in this case happens every time getInstance() is called because it's synchronized.

like image 217
Matt Quigley Avatar asked Aug 12 '13 22:08

Matt Quigley


3 Answers

You are absolutely correct.

There is no guarantee that writes in one Thread will be visible is another.

To provide this guarantee you would need to use the volatile keyword:

private volatile String someData;

Incidentally you can leverage the Java classloader to provider thread safe lazy init of your singleton as documented here. This avoids the synchronized keyword and therefore saves you some locking.

It is worth noting that the current accepted best practice is to use an enum for storing singleton data in Java.

like image 192
Boris the Spider Avatar answered Nov 12 '22 23:11

Boris the Spider


Correct, it is possible that thread 1 still sees the value as being "one" since no memory synchronization event occurred and there is no happens before relationship between thread 1 and thread 2 (see section 17.4.5 of the JLS).

If someData was volatile then thread 1 would see the value as "two" (assuming thread 2 completed before thread 1 fetched the value).

Lastly, and off topic, the implementation of the singleton is slightly less than ideal since it synchronizes on every access. It is generally better to use an enum to implement a singleton or at the very least, assign the instance in a static initializer so no call to the constructor is required in the getInstance method.

like image 28
Trevor Freeman Avatar answered Nov 12 '22 23:11

Trevor Freeman


Isn't it possible that the last value of value in thread 1 can still be "one"?

Yes it is. The java memory model is based on happens before (hb) relationships. In your case, you only have getInstance exit happens-before subsequent getInstance entry, due to the synchronized keyword.

So if we take your example (assuming the thread interleaving is in that order):

// Thread 1
GlobalData d = GlobalData.getInstance(); //S1
d.setData("one");

// Thread 2
GlobalData d = GlobalData.getInstance(); //S2
d.setData("two");

// Thread 1
String value = d.getData();

You have S1 hb S2. If you called d.getData() from Thread2 after S2, you would see "one". But the last read of d is not guaranteed to see "two".

like image 21
assylias Avatar answered Nov 13 '22 01:11

assylias