CODEWITHSUNDEEP
java
Kai
Kai

Reputation: 3865

Singleton in singleton

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:

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

Thanks!

Upvotes: 1

Views: 1376

Answers (5)

Ishan Nimantha Ish
Ishan Nimantha Ish

Reputation: 1

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.

Upvotes: 0

dagnelies
dagnelies

Reputation: 5329

....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;
    }
}

Upvotes: 0

Eric Lindauer
Eric Lindauer

Reputation: 1837

  1. it's ok to add some code to your private, no-args constructor.

  2. 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.

  3. 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.

Upvotes: 2

George
George

Reputation: 1465

  1. Not sure, what you mean. It is certainly syntactically acceptable to have code in the Singleton constructor

  2. 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.

  3. 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.

Upvotes: 0

Garrett Hall
Garrett Hall

Reputation: 30042

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;
    }
}

Upvotes: 1

Related Questions