0
public static Database getInstance()
{
    if( instance == null )
    {
        return instanceNotFoundDoubleCheckSynced();
    }
    return instance;
}


synchronized private static Database instanceNotFoundDoubleCheckSynced()
{
    if( instance == null )
    {
        Connection connection = establishConnection();
        if( connection != null )
        {
            instance = new Database( connection );
        }

    }

The above code first tries to get an instance of the database connection. If it cannot do so, it then enters a synchronized method which checks again and if there still is no instance it will create a new one.

The reason I have two methods checking this is because the first one is not synced and lots of threads can run it simultaneously, but I dont want threads running the method that actually creates a new connection simultaneously.

My problem is that once the threads reach the instanceNotFoundDoubleCheckSynced method they all wait for their chance to run it.

Can anyone suggest how I could get around this? I dont think it would be a problem on a production environment since it would be unlikely that so many calls would come to this method before a database connection is established, but in my JUnits I have 10,000 threads firing off and they are all getting stuck.

Thanks

2
  • Singletons are becoming bad practise these days. I wouldn't recommend implementing a Singleton solution for your project. Oct 25, 2014 at 15:59
  • 1
    If at all you are going ahead with this design. One advice would be to have your code run on startup. If it happens to be web app you can do it by creating a listener. Also think of the case as to what will happen when remote server is restared. Since this is singleton will you have to restart your jvm ? Oct 25, 2014 at 16:34

2 Answers 2

0

You should avoid double check locking because it is very easy not to do it right. I think it would be better if you used Initialization-on-demand holder idiom, where you just instanciate the shared object directly in an inner class. It will be simpler and more efficient.

0

I think the problem is that you are testing an unrealistic scenario where all threads start and try to get a connection. Because a) getting a connection takes time b) there are so many threads starting simultaneously, they all manage to get pass the first unsynchronized check and begin to wait on the synchronized call. I'd recommend setting the number of test threads to a realistic number (100-200) and adding a random delay when starting threads.

Also, you can simplify your double-checking into a single method and add a proper error handling:

class MyClass {

   public static Database getInstance()
   {
       if( instance == null )
       {
           synchronized (MyClass.class) 
           {
             if( instance == null )
             {
                 Connection connection = establishConnection();
                 if( connection == null )
                 {
                     throw new RuntimeException("Cannot establish connection");
                 }
             }

             instance = new Database( connection );
          } 
       }

      return instance;
   }
}

Hope this helps.

Slava Imeshev

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.