Singleton in singleton

Question

I have a singleton in singleton data structure. Currently my implementation is like below:

public class Singleton {
    private Object embInstance;

    private Singleton() { 
        embInstance = new Object();
    }

    private static class SingletonHolder { 
            public static final Singleton instance = new Singleton();
    }

    public static Singleton getInstance() {
            return SingletonHolder.instance;
    }

    public Object getEmbInstance() {
        return embInstance;
    }

    public Object resetEmbInstance() {
        embInstance = null;
    }

}

My question are:

Does 'private Singleton()' have to be empty or is it OK to add some code in it?
Is this implementation thread-safe with respect to embInstance?
This implementation is not lazy-loading for embInstance. How to implement a thread-safe lazy-loading for embInstance?

Thanks!

What is the purpose of `resetEmbInstance()`? It would seem to be stuck as `null` after calling it once. — Paul Bellora, Sep 26 '11 at 23:29
This is called the "initialization on demand holder" idiom: http://en.wikipedia.org/wiki/Singleton_pattern#The_solution_of_Bill_Pugh — BalusC, Sep 26 '11 at 23:35
possible duplicate of [Singleton class in java](http://stackoverflow.com/questions/2111768/singleton-class-in-java) — Brian Roach, Sep 26 '11 at 23:36
@BrianRoach: It has nothing to do with the suggested dupe. The OP is referring a nested Singelton class in here. — amit, Sep 26 '11 at 23:37
Based on what I can see in you code you do not have a singleton in singleton. — Shahzeb, Sep 26 '11 at 23:43
@amit - if you bothered to read the accepted answer to the other question... — Brian Roach, Sep 26 '11 at 23:57

score 2 · Answer 1 · answered Sep 26 '11 at 23:44

it's ok to add some code to your private, no-args constructor.
I believe the SingletonHolder class will only be initialized once, therefore instance will be guaranteed to be assigned exactly once. However, within the Singleton class you have setters and getters that are not synchronized, so you may have some threading issues there wrt embInstance.

thread-safe lazy-loading: same as lazy-loading, but do it inside a synchronized block:

public static Object getInstance() {
    if(embInstance == null) {
        synchronized(Singleton.class) {
            if(embInstance == null) {
                embInstance = new Singleton();
            }
        }
    }

    return embInstance;
}

Note that this code is both efficient (no synchronization required once your data is initialized) and thread-safe (initialization occurs inside a synchronized block).

Hope that helps.

Of course you can synchronize on any convenient object, Singleton.class is just one choice. — Eric Lindauer, Sep 26 '11 at 23:49
It's to avoid the synchronization overhead once the variable has been initialized. It's there to show the most efficient implementation, but if you dropped it for readability, I'd support that. :) — Eric Lindauer, Sep 27 '11 at 00:06

score 1 · Accepted Answer · answered Sep 26 '11 at 23:40

1

The synchronized keyword keeps the methods thread-safe. The getEmbInstance() is the standard way to do lazy instantiation.

public class Singleton {
    private Object embInstance;

    private Singleton() { 
    }

    private static class SingletonHolder { 
            public static final Singleton instance = new Singleton();
    }

    public static Singleton getInstance() {
            return SingletonHolder.instance;
    }

    public synchronized Object getEmbInstance() {
        if (embInstance == null)
            embInstance = new Object();
        return embInstance;
    }

    public synchronized Object resetEmbInstance() {
        embInstance = null;
    }
}

answered Sep 26 '11 at 23:40

Garrett Hall

29,524
10
61
76

It is not enough, two threads can 'ask' for `embInstance`, and one might modify a certain value before the second had the time to read it. also, you probably want to add the `volatile` keyword to the `embInstance` declaration or bad things might happen.. – amit Sep 26 '11 at 23:44
I don't get it. I think Gnon's implementation is good, since getEmbInstance() is synchronized. – Kai Sep 26 '11 at 23:53

score 0 · Answer 3 · edited Mar 26 '18 at 07:36

0

public class A {
    A a;
    private A() {}

    synchronized public static A getOb() {
        if (a == null) {
            a = new A();
        }
        return a;
    }
}

You should use synchronized for thread-safety in Singleton.

edited Mar 26 '18 at 07:36

shmosel

49,289
6
73
138

answered Mar 26 '18 at 07:34

Ishan Nimantha Ish

1
2

dagnelies · Answer 4 · 2011-09-26T23:54:28.303

0

....eh ...isn't that overcomplexifying things? Why have a singleton in a singleton? Why so many classes?

Moreover, you have a method to 'reset' your singleton to null, but none to actually instantiate a new one.

Why not simply:

public class Singleton {
    private static Singleton instance;

    private Singleton() { }

    public static synchronized Singleton getInstance() {
              if (instance == null)
                     instance = new Singleton(); // The "lazy loading" :p
          return instance;
    }
}

edited Sep 26 '11 at 23:54

answered Sep 26 '11 at 23:41

dagnelies

5,203
5
38
56

I believe the reason is to allow for lazy initialization. You may not want to allocate that instance object. If you do, your only option is to make it null until it's initialized. By placing it in a holder class, you're able to make it final while at the same time delaying initialization until access time. – corsiKa Sep 26 '11 at 23:46
I agree with arnaud, the singleton-in-a-singleton pattern is pretty strange. I'd try to refactor the code to avoid this. – Eric Lindauer Sep 26 '11 at 23:46
@glowcoder: Well, in the "simple" example, object creation occurs only when the singleton is actually requested. Moreover, despite it is not final, you have no way to modify it from outside the class, which is kindof the same as final to me ...and actually, the OP *wants* to be able to change it to null at some point. – dagnelies Sep 26 '11 at 23:50
@arnaud it being `final` has important implications. For example, some junior programmer comes along and tries to reassign it in two years. This bug is detected immediately with it being final, as you know at compile time. Your method makes it determined when it fails at best during testing, probably during production, and at worst never. – corsiKa Sep 27 '11 at 01:27

score 0 · Answer 5 · answered Sep 26 '11 at 23:42

Not sure, what you mean. It is certainly syntactically acceptable to have code in the Singleton constructor
This is not inherently thread safe. If embInstance is accessed by multiple threads even if it is via getEmbInstance, it will suffer from race conditions. You must use synchronized to access it if you wish for threadsafety.
To implement a thread-safe lazy load, you must have a lock that prevents changes to Singleton. You can just use the instance of Singleton to lock it as such:

synchronized(this)

Also, you don't need SingletonHolder you can just have a public static Singleton instance in the Singleton class itself.

Singleton in singleton

5 Answers5