How to achieve synchronisation in a better way with singleton approach?

Question

Class which is implemented with Singleton Pattern is as follows, when multiple threads access this method only one thread has to create the instance so all I am doing is synchronising the method

private static synchronized FactoryAPI getIOInstance(){
    if(factoryAPI == null){
        FileUtils.initWrapperProp();
        factoryAPI =  new FactoryAPIImpl();
    }
    return factoryAPI;
}

which I feel is unnecessary because only for the first time the instance would be created and for the rest of the time the instance created already would be returned. When adding synchronised to block allows only one thread to access the method at a time.

The getIOInstance does two jobs

i) Initialising properties and

ii) Creating a new instance for the first time

So, I'm trying to do block level synchronisation here like the following

private static FactoryAPI getIOInstance(){
    if(factoryAPI == null){
        synchronised {
            if(factoryAPI == null){
                FileUtils.initWrapperProp();
                factoryAPI =  new FactoryAPIImpl();
             }
        }
    }
    return factoryAPI;
}

I prefer the second one to be the right one. Am I using it in a right way? Any suggestions are welcome.

Duplicate: find solution at below link: https://stackoverflow.com/questions/20906548/why-is-synchronized-block-better-than-synchronized-method — Rahul Kumar, Jul 11 '17 at 04:46
The second method is called [double-checked locking](https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java) and only works correctly if you follow a precise formula. Consider the [initialization-on-demand holder idiom](https://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom) as a safer alternative. — shmosel, Jul 11 '17 at 04:51

score 0 · Answer 1 · edited Jul 11 '17 at 05:10

0

Use the first method because the second one is not thread-safe.

When you say,

factoryAPI = new FactoryAPIImpl();

The compiler is free to execute the code in the following order:

1) Allocate some memory on the heap
2) Initialize factoryAPI to the address of that allocated space
3) Call the constructor of FactoryAPIImpl

The problem is when another thread calls getIOInstance() after step 2 and before step 3. It may see a non-null factoryAPI variable that points to an uninitialized FactoryAPI instance.

edited Jul 11 '17 at 05:10

answered Jul 11 '17 at 04:42

Evan Darke

604
4
7

The visibility issue can be avoided by making the field `volatile`. – shmosel Jul 11 '17 at 04:45

score 0 · Answer 2 · answered Jul 11 '17 at 06:36

0

There are many different answers to this problem, you can find an extensive discussion at SEI for example.

The modern Java solution is simple: use an enum - as the JLS guarantees you that the compiler / JVM will create exactly one thing.

answered Jul 11 '17 at 06:36

GhostCat

137,827
25
176
248

It sounds like OP is specifically looking for a lazy-loaded singleton. – shmosel Jul 12 '17 at 20:59

score 0 · Answer 3 · answered Jul 11 '17 at 07:37

Found Initialization-on-demand holder method of initialising as an interesting one like the following,

public class FactoryAPI {
    private FactoryAPI() {}

    private static class LazyHolder {
        static final Something INSTANCE = new Something();
    }

    public static Something getInstance() {
        return FactoryAPI.INSTANCE;
    }
}

Since the class initialization phase is guaranteed by the JLS to be serial, i.e., non-concurrent, no further synchronization is required in the static getInstance method during loading and initialization.

And since the initialization phase writes the static variable INSTANCE in a serial operation, all subsequent concurrent invocations of the getInstance will return the same correctly initialized INSTANCE without incurring any additional synchronization overhead.

How to achieve synchronisation in a better way with singleton approach?

3 Answers3