singleton synchronized doubt

[vanlalhmangaiha khiangte]

Hi all,
I have a singleton class which has a method as below

    
/**
 * To get the singleton instance of SingleTon
 * @return SingleTon
 */
public static SingleTon getInstance()
{
if (instance == null)
{
    synchronized (SingleTon.class)
    {
        if( instance == null )
        {
        instance = new SingleTon();
        }
    }
}
return instance;
}

As you can seen from the code there are two check
if( instance == null )
The first check i understood.
The second one is the one that i am having a doubt..

Multiple threads can access this class and

    
 
    synchronized (SingleTon.class)
    {
        if( instance == null )
        {
        instance = new SingleTon();
        }
    }
 

This code above ensures that if the same thread access this class again,then it will return the same object else it will create a new one.

If i am wrong please tell me why there are two checks for
if( instance == null )

And is this check necessary for multiple threads ..

Thanks in advance,

[arulk pillai]

You need the second check. Let us take a scenario where there are two threads concurrently accessing your getInstance method


Thread 1: Gets the lock on SingleTon.class and creating an instance.

Thread 2: Has passed the outer if(instance == null) and waiting for the lock. Once the lock is released by the first thread it goes inside. If we do not re-check if(instance == null) we may create a new instance. The thread 1 has already created an instance.

[Paul Beckett]

the getInstance method is using the "double checked locking" principle.

The idea is (in pseudocode):

if condition is true
   acquire a lock on the object
      recheck that condition is still true
        do something

The way it is used in the getInstance method looks like it is for performance. There is no point in using the resources to get a lock on the Singleton.class if the singleton instance has already been created.

By the way, apparently double checked locking doesn't always work in java. I'm no expert on this but see here for more info

[arulk pillai]

You might as well synchronize on the method level. you are not doing any intensive operations in your getInstance method.

[Rob Spoor]

Although I agree it is easier, there is most definitely a (small) performance hit when synchronizing the entire method.

If instance != null, you don't need to enter the synchronized block at all. If the entire method is synchronized you will synchronize unnecessarily. Since synchronization has some overhead synchronzing the entire method will be slower.

[victor kamat]

You need to make instance static volatile.

Also this is discussed by Bloch in Effective Java.

[Paul Beckett]

as far as I'm aware, marking it as volatile can be done in earlier versions of java but only works correctly for double checked locking in 1.5 and later.

[vipin raimcs]

that's true that there would be performance hit if we used synchronization over entire method so its better to use synchronize block and do the checks which have done before for safety.