I happened upon an article recently discussing the double checked locking pattern in Java and its pitfalls and now I'm wondering if a variant of that pattern that I've been using for years now is subject to any issues.
I've looked at many posts and articles on the subject and understand the potential issues with getting a reference to a partially constructed object, and as far as I can tell, I don't think my implementation is subject to these issues. Are there any issues with the following pattern?
And, if not, why don't people use it? I've never seen it recommended in any of the discussion I've seen around this issue.
public class Test {
private static Test instance;
private static boolean initialized = false;
public static Test getInstance() {
if (!initialized) {
synchronized (Test.class) {
if (!initialized) {
instance = new Test();
initialized = true;
}
}
}
return instance;
}
}
First, for singletons you can use an Enum, as explained in this question Implementing Singleton with an Enum (in Java)
Second, since Java 1.5, you can use a volatile variable with double checked locking, as explained at the end of this article: https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html
You should probably use the atomic data types in java.util.concurrent.atomic.
There are still some cases when a double check may be used.
final
field set at the end of the constructor/initialized block (that causes all previously initialized fields to be seen by other threads).Double check locking is broken. Since initialized is a primitive, it may not require it to be volatile to work, however nothing prevents initialized being seen as true to the non-syncronized code before instance is initialized.
EDIT: To clarify the above answer, the original question asked about using a boolean to control the double check locking. Without the solutions in the link above, it will not work. You could double check lock actually setting a boolean, but you still have issues about instruction reordering when it comes to creating the class instance. The suggested solution does not work because instance may not be initialized after you see the initialized boolean as true in the non-syncronized block.
The proper solution to double-check locking is to either use volatile (on the instance field) and forget about the initialized boolean, and be sure to be using JDK 1.5 or greater, or initialize it in a final field, as elaborated in the linked article and Tom's answer, or just don't use it.
Certainly the whole concept seems like a huge premature optimization unless you know you are going to get a ton of thread contention on getting this Singleton, or you have profiled the application and have seen this to be a hot spot.
If "initialized" is true, then "instance" MUST be fully initialized, same as 1 plus 1 equals 2 :). Therefore, the code is correct. The instance is only instantiated once but the function may be called a million times so it does improve the performance without checking synchronization for a million minus one times.