Singleton pattern with combination of lazy loading and thread safety

Question

I was doing some research about singletons, specifically regarding lazy vs eager initialization of singletons.

An example of eager initialization:

public class Singleton
{
    //initialzed during class loading
    private static final Singleton INSTANCE = new Singleton();

    //to prevent creating another instance of Singleton
    private Singleton(){}

    public static Singleton getSingleton(){
        return INSTANCE;
    }
}

but as shown above that it is eager initialization and thread safety is left to jvm but now, I want to have this same pattern but with lazy initialization.

so I come up with this approach:

public final class Foo {
    private static class FooLoader {
        private static final Foo INSTANCE = new Foo();
    }
    private Foo() {
        if (FooLoader.INSTANCE != null) {
            throw new IllegalStateException("Already instantiated");
        }
    }
    public static Foo getInstance() {
        return FooLoader.INSTANCE;
    }
}

As shown above Since the line

private static final Foo INSTANCE = new Foo();

is only executed when the class FooLoader is actually used, this takes care of the lazy instantiation, and is it guaranteed to be thread safe.

Is this correct?

This is not really lazy initialization though? The instance can be loaded prior to the `getInstance` method being invoked. — Perception, Apr 03 '13 at 16:00
This is certainly lazy. The `INSTANCE` is only instantiated on first reference of the class. Which will only be referenced in the first invocation of `getInstance`. Though I would remove the creation check in the constructor. — John Vint, Apr 03 '13 at 16:06
@JohnVint - tell me, what would happen if I did this - `Class.forName("Foo")`? — Perception, Apr 04 '13 at 00:23
it will create the instace of the Foo class. That initialixation includes static fields and thr static block. It does not include static child classes. you can try it yourself to see. — John Vint, Apr 04 '13 at 01:35

score 26 · Accepted Answer · edited Jan 06 '16 at 16:01

26

Your second code snippet is, in my opinion, the best way of thread-safe lazily initializing a singleton. It actually has a pattern name

Initialization-on-demand holder idiom

I would suggest you use it.

edited Jan 06 '16 at 16:01

Josiah Yoder

3,321
4
40
58

answered Apr 03 '13 at 16:03

John Vint

39,695
7
78
108

Thanks a lot. gone through the article perfect explnation – tuntun fdg Apr 03 '13 at 16:46
Replying on the class loading behavior of the JVM rather than making your laziness explicit sounds to me like a bad idea. There's probably even no performance benefit compared to the straightforward way. – einpoklum Apr 04 '13 at 09:16
@einpoklum I saw your example used DCL + volatile. Though it is correct (wrt declaring it volatile ) I would rather use his latter example then the DCL. I find it to be more readable and less verbose. Lastly your example is wrong, you should be synchronizing on `Foo.class` and not the instance. – John Vint Apr 04 '13 at 12:10
I agree this is the best solution. It is [faster than the double-checked lock](http://literatejava.com/jvm/fastest-threadsafe-singleton-jvm/). It has less conceptual baggage than the Enum Way, and works in exactly the same way. ([Unless getInstance is the only static method, then use the OP's first code snippet.](http://stackoverflow.com/a/15793074/1048186)) – Josiah Yoder Jan 06 '16 at 15:46

score 10 · Answer 2 · answered Apr 03 '13 at 16:35

10

You first design is actually lazy. Think about it, the instance is only created when the class is initialized; the class is only initialized when the getSingleton() method is called [1]. So the instance is only created when it's asked for, i.e. it's lazily created.

[1] http://docs.oracle.com/javase/specs/jls/se7/html/jls-12.html#jls-12.4.1

answered Apr 03 '13 at 16:35

ZhongYu

19,446
5
33
61

+1 you are right it is still lazy though I imagine he would have other methods available in which a different static reference would create it. – John Vint Apr 03 '13 at 16:52
It's unlikely though that a singleton class contains static methods (other than `getInstance()`) – ZhongYu Apr 03 '13 at 18:11
2

but it is not unlikely that it contains a public static field :) – Kostas Kryptos Jan 07 '15 at 16:40

Mikhail · Answer 3 · 2013-04-03T16:21:00.687

3

The second one is very bad in terms of readability, first one is suitable. Have a look at this article. Its about double check locking, but also will give you wide information about singletons multithreading.

edited Apr 03 '13 at 16:21

answered Apr 03 '13 at 16:01

Mikhail

4,175
15
31

How is the second one *very bad*? – John Vint Apr 03 '13 at 16:13
It will work, but I do not see any use in FooLoader and if statement inside constructor. Let it be not bad but overburden. Remember other people will read your code. So keep it simple. – Mikhail Apr 03 '13 at 16:18
I agree, have the check in the constructor is not necessary and should be removed. But when talking about thread-safety I see absolutely nothing wrong with the second snippet of code. – John Vint Apr 03 '13 at 16:22
In both cases new instance is created while class initialization, so no difference. – Mikhail Apr 03 '13 at 16:26
That is not correct. Try it yourself. Have the constructor of the singleton type class do some println. Now have another static method do a different println method. If you were right, then you should see both printlns if you never invoke the `getInstance` and only invoke the other method that does a simple println – John Vint Apr 03 '13 at 16:47

score 1 · Answer 4 · answered Apr 03 '13 at 16:00

1

The best way is actually to use the Enum Way:

public enum Singleton {
    INSTANCE;
    public void execute (String arg) {
            //... perform operation here ...
    }
}

answered Apr 03 '13 at 16:00

Jean Logeart

52,687
11
83
118

2

I would say it's the worst way... and Bloch should refund his readers:) – ZhongYu Apr 03 '13 at 16:32
+1 zhong.j.yu I too never liked this pattern even though Bloch talked about it so highly and then everyone else followed it :/ – John Vint Apr 03 '13 at 16:51
who have deleted 'enum way' from the wiki page!? :) – Mike Oct 08 '21 at 08:41

einpoklum · Answer 5 · 2021-02-12T09:32:08.687

In my opinion, this is an inappropriate pattern to use. It makes assumptions about the JVM's behavior which are non-trivial and confusing. Also, it has a dummy class. Dummy classes should be avoided when possible.

I suggest the straightforward approach:

public class Foo {
    private volatile static final Foo instance = null;

    private Foo() { }

    public static Foo instance() {
        if (instance == null) instance = new Foo();
        return instance;
        }
    }
}

... although, this does not work as-is - it's not thread safe.. What you really want is the double-check pattern presented in Item 71 of Bloch's Effective Java; see here. Adapting the example at the link to your case, we get:

public class Foo {
    private volatile static final Foo instance = null;

    private Foo() { }

    public static Foo instance() {
        if (instance != null) return instance;
        synchronized(instance) {
           Foo result = instance;
           if (instance == null) {
                result = instance = new Foo();
           return result;
        }
    }
}

Notes:

Don't worry about the performance of this code, modern JVMs take care of it and it's just fine. After all, premature optimization is the root of all evil.
As is suggested in other answers, the above is not Bloch's preferred solution, but I think using an enum for a singleton is semantically inappropriate just like what OP did originally.

If you aren't worried about performance, just use your first example and synchronize the whole method. Even simpler! — Josiah Yoder, Jan 06 '16 at 15:16
For alternative versions of the double-lock, see https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_Java — Josiah Yoder, Jan 06 '16 at 15:32

Singleton pattern with combination of lazy loading and thread safety

5 Answers5

Linked

Related