I've implemented the singleton pattern for a data access object. Do I have to synchronize the "getInstance()" method of that singleton? Please have a look at the provided code and tell me if two threads accessing that method simultaneously can create two instances of that class.
So what can happen is that two threads are entering line "X" and creating an instance of X. An even worse case is that thead A creates an instance of X and returnes it and after that thread B creates an instance of X and returns it. B and all following threads are working with the instance of X which thread B created, but thread A works with his own instance.
Are you sharing my opinion?
If yes, I think there are two solutions of that problem:
1. Synchronize on the class instance of X
2. Creating the singleton instance in a static constructor
So the instance is created while the classloader loads the class. Would this prevent a second thread loading the same class at the same time from overwriting the reference to the first instance?
A even worse feeling you can get if you decompile such singletons and have a look at the byte code because here you can see that the Java commands used are far away from beeing atomic Here the examples from above:
So I hope you have some suggestions to the solutions 1 and 2.
There's an short and a long answer to your questions, I'll try a long one . Your original code is definately not thread-safe, two threads can both succesfully pass the if statement and make seperate instances as you've mentioned.
Using the synchronized keyword does make the getInstance method thread-safe, but at the cost of some performance since the synchronization will occur each time the method is called (which in the case of singletons may be quite often). It's up for debate if this is a consideration you should worry about for your SJCD assignment though, since clarity is more important than performance.
If you do think it's an issue you can either use eager initialisation (as opposed to the lazy initialisation in getInstance() ). On that note, i personally prefer :
but that's definately a taste issue. Another often used approach is the double null check approach, which uses an unsynchronized early out and avoids every call entering the synchronized block :
That's a fast and thread-safe implementation of lazy initialisation. But if you want my personal preference I'd go for eager initialisation. It's cleaner code.
Hope that helped. [ November 16, 2007: Message edited by: R van Vliet ]