Solved

Thread Safe Singleton?

Posted on 2010-09-23
13
1,422 Views
Last Modified: 2012-05-10
I am looking at creating a thread safe singleton, I have put PMD Rules in place and my code (See below)

When I run the PMD Rules it's saying that :

double checked locking is not thread safe in java

and that my singleton is not thread safe.

Does anyone have a thread safe singleton or links to one?

Cheers



public class LogRegistry {



	private static LogRegistry instance = null;

	private static Object syncObject;



	private LogRegistry()

	{

	}



	public static LogRegistry getLogRegistry()

	{

if(instance == null)

		{

		synchronized(syncObject)

		{

			if(instance == null)

			{

				instance = new LogRegistry();

			}

		}

}



		return instance;

	}

}

Open in new window

0
Comment
Question by:directxBOB
  • 6
  • 4
  • 2
  • +1
13 Comments
 
LVL 20

Expert Comment

by:ChristoferDutz
ID: 33742350
You are using a syncObject of value null ...
I usually have a String as syncObject. As long as this is 0 I doubt synchronization will work at all.
public class LogRegistry {



        private static LogRegistry instance = null;

        private static Object syncObject = "mySyncObject";



        private LogRegistry()

        {

        }



        public static LogRegistry getLogRegistry()

        {

if(instance == null)

                {

                synchronized(syncObject)

                {

                        if(instance == null)

                        {

                                instance = new LogRegistry();

                        }

                }

}



                return instance;

        }

}

Open in new window

0
 
LVL 10

Assisted Solution

by:Hegemon
Hegemon earned 50 total points
ID: 33742416
You can add another class, LogRegistryFactory, in which you will create your singleton in static initialisation. Thus: 1) you are guaranteed to have the only instance of the registry; 2) you separate the concerns of LogRegistry creation and use; 3) No need in synchronisation.

public class LogRegistryFactory {

	private static LogRegistry instance = new LogRegistry();

	public static LogRegistry getLogRegistry() {
             return instance;
        }
}

Open in new window

0
 
LVL 20

Accepted Solution

by:
ChristoferDutz earned 250 total points
ID: 33742444
The level to which this approach still is not 100% thread safe is relatively minimal. The only problem you may have is that if Thread A currently is creating "instance" and Thread B is called while the object is still created. It could happen, that the memory has allready been assigned to the varieable (and therefore it is not null) but the constructor is not finished initializing, so using it could result in errors.

One solution would be not to initialize the "instance" lazyly. If you create it as soon as the class is loaded you won't have any problems with it. Another solution would be method level synchronization, but synchronization is extremely expensive.

Here's a good article about the problem:
http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double.html?page=4

The following code should be quite thread-safe.
public class LogRegistry {



        private static final LogRegistry instance = new LogRegistry();



        private LogRegistry()

        {

        }



        public static LogRegistry getLogRegistry()

        {

            return instance;

        }

}

Open in new window

0
 
LVL 26

Assisted Solution

by:ksivananth
ksivananth earned 200 total points
ID: 33742520
you can achieve thread safe lazy loading singleton using an inner class,

public class LogRegistry {

       
        private LogRegistry()
        {
        }

        public static LogRegistry getLogRegistry()
        {
            return LogRegistryInstanceHolder.instance;
        }

   private class LogRegistryInstanceHolder{
private static final LogRegistry instance = new LogRegistry();

   }
}
0
 

Author Comment

by:directxBOB
ID: 33742705
@ChristoferDutz: unfortunately the double-checked locking isn't thread safe in java.

@Hegemon: like this idea and will keep in mind if this project expands further.

@ChristoferDutz: this is what I am leaning towards now, simply create a static final instance the second it gets called. My question is, if I do it this way do I really need a getLogRegistry?

as

LogRegistry.SomeMethod() would do the exact same thing?


@ksivananth: I think that ChristoferDutz method above may work easier. Not that there is a huge overhead in creating an inner class, it just seems redundant?

New to java so how the JVM and compiler works is all new.

Cheers


0
 
LVL 20

Expert Comment

by:ChristoferDutz
ID: 33742728
Well you asked for a Singleton ;-) ...

of course you may use static methods. But I guess it depends on the thread-Safety of the methods you are using. The above pattern only avaoids concurrency problems when retrieving instances, if the methods of the instance are not thread safe,you will get problems there.
0
Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

 
LVL 26

Expert Comment

by:ksivananth
ID: 33742730
>>it just seems redundant?

it depends, if you really require( based on the complexity of creation and usage ) lazy loading, thats the way without sync.
0
 
LVL 20

Expert Comment

by:ChristoferDutz
ID: 33742804
I think there is no real difference between ksivananths and my solution. The only thing is that instance variable and getInstance method are in two classes in his approach and in one in mine. Anyway I think I wouldn't have posted mine if I had allready seen his solution when I started writing ;-)
0
 
LVL 20

Expert Comment

by:ChristoferDutz
ID: 33742854
@ksivananth the problem with your approach in 33742520 is that the synchronized block is allways executed. Synchronization is an extremely expensive operation (I have read about 100 times the overhead of a normal method call). Thats the main reason for the double checked locking. By synchronizing the access method you could skip the holder class completely and simply use the classic Singleton pattern. But if you have to gain access to an instance of LogRegistry often from different threads, this will get you into realy big performance trouble.
0
 
LVL 26

Expert Comment

by:ksivananth
ID: 33742982
>> think there is no real difference between ksivananths and my solution.

there is a big difference, yours will create the instance when the class loaded into memory, mine will create only when the getinstance accessed!

>>@ksivananth the problem with your approach in 33742520 is that the synchronized block is allways executed.

there is not synchronization block!

>>Synchronization is an extremely expensive operation (I have read about 100 times the overhead of a normal method call).

I know, thats why suggested the inner class approach

>>Thats the main reason for the double checked locking

DCL is something you should never use, thats the reason for the big article in JavaWorld!
0
 
LVL 20

Expert Comment

by:ChristoferDutz
ID: 33743537
Lol .. sorry for that .. I was refering to the solution of Hegemon ... didn't see that this was somebody different that posted the possible solution.

Ok ... and you are correct ... but to be 100% precise ... to me it looks as if you are not immune against concurrency issues against first access of the component from two threads simultaneously. Do you have any References stating that the solution you posted actually works as you stated it? I could read that none of the solutions that do lazy-loading actually work. in the JavaWorld article they state that eager initialization currently seems to be the only 100% safe method.
0
 
LVL 26

Expert Comment

by:ksivananth
ID: 33751267
the idea is that static inner class is loaded only when referenced which we do here only in the getInstance! here you go http://c2.com/cgi/wiki?DoubleCheckedLockingIsBroken
0
 

Author Closing Comment

by:directxBOB
ID: 33752224
Thanks for all the help, split the points over the 3 solutions that I tested with. I like the factory idea but will probably opt for the one of the other 2 options once I see what testing throws up.
0

Featured Post

Is Your Active Directory as Secure as You Think?

More than 75% of all records are compromised because of the loss or theft of a privileged credential. Experts have been exploring Active Directory infrastructure to identify key threats and establish best practices for keeping data safe. Attend this month’s webinar to learn more.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

Title # Comments Views Activity
Free Alternative to JIRA 4 90
what is the difference between "sudo su" and "su - root" 6 102
Error trying to install RTMT Win7 5 38
xampp tool 12 23
For beginner Java programmers or at least those new to the Eclipse IDE, the following tutorial will show some (four) ways in which you can import your Java projects to your Eclipse workbench. Introduction While learning Java can be done with…
Introduction This article is the second of three articles that explain why and how the Experts Exchange QA Team does test automation for our web site. This article covers the basic installation and configuration of the test automation tools used by…
Video by: Michael
Viewers learn about how to reduce the potential repetitiveness of coding in main by developing methods to perform specific tasks for their program. Additionally, objects are introduced for the purpose of learning how to call methods in Java. Define …
Viewers will learn one way to get user input in Java. Introduce the Scanner object: Declare the variable that stores the user input: An example prompting the user for input: Methods you need to invoke in order to properly get  user input:

930 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

13 Experts available now in Live!

Get 1:1 Help Now