Reputation: 1703
I am trying to find my own way of Java Singleton implementation. The code is as follows:
public class Singleton{
private volatile static Singleton _instance = null;
private Singleton(){}
public static Singleton getInstance(){
if (_instance == null)
Object obj = new Object();
synchronized(obj){
if (_instance == null)
_instance = new Singleton();
}
return _instance;
}
Does this code work? If not work, how to fix it?
Upvotes: 1
Views: 621
Reputation: 136112
No, it's not correct, you should synchronize on Singleton.class
class Singleton {
private volatile static Singleton _instance;
private Singleton(){}
public static Singleton getInstance(){
if (_instance == null)
synchronized(Singleton.class){
if (_instance == null)
_instance = new Singleton();
}
return _instance;
}
}
it's a known double-checked locking pattern, see http://www.ibm.com/developerworks/java/library/j-dcl/index.html for details
Note that since there's only one method in this class the following serves the same purpose without any double-checked tricks and it's lazy too
class Singleton {
private static Singleton _instance = new Singleton();
private Singleton(){}
public static Singleton getInstance(){
return _instance;
}
}
Upvotes: 0
Reputation: 425318
No - your code doesn't work, because the lock object is a local variable and therefore different for every thread.
You are attempting to implement the lazy initialization pattern - where the instance is created when first used.
But there is a simple trick that allows you to code a thread-safe implementation that doesn't require synchronization! It is known as the Initialization-on-demand holder idiom, and it looks like this:
public class Singleton {
private static class Holder {
static final Singleton INSTANCE = new Singleton();
}
public static Singleton getInstance() {
return Holder.INSTANCE;
}
private Singleton() {
}
// rest of class omitted
}
This code initializes the instance on the first calling of getInstance()
, and importantly dosen't need synchronization because of the contract of the class loader:
Holder
's only access is within the getInstance()` method)Holder
's static block fires)It's a neat little trick that I use whenever I need lazy initialization. You also get the bonus of a final
instance, even thought it's created lazily. Also note how clean and simple the code is.
Upvotes: 3
Reputation: 328855
When you write:
Object obj = new Object();
synchronized(obj){}
The JVM can prove that no two threads can acquire that lock (because it is a local variable) and can therefore completely remove the synchronization.
A few comments about singletons:
Upvotes: 1
Reputation: 1278
Ticky and really simple implementation of not-lazy singleton:
public enum TickySingleton {
INSTANCE;
public void doSomething() { ... }
public Object returnSomething() { ... }
}
}
Not everybody will like this. ;)
Upvotes: 2
Reputation: 11559
it would be better if your synchronization object is final static
. Otherwise each possible concurrent thread will create its own sync object and lock different objects. And they will not wait for each other.
public class Singleton{
private final static Object obj = new Object();
private volatile static Singleton _instance = null;
private Singleton(){}
public static Singleton getInstance(){
if (_instance == null)
synchronized(obj){
if (_instance == null)
_instance = new Singleton();
}
}
return _instance;
}
Upvotes: 1