Double Checked Locking is this idiom:
// Broken -- Do Not Use!
class Foo {
private Helper helper = null;
public Helper getHelper() {
if (helper == null) {
synchronized(this) {
if (helper == null) {
helper = new Helper();
}
}
}
return helper;
}
The point of this code is to avoid synchronization when the object has already been constructed. This code doesn't work in Java. The basic principle is that compiler transformations (this includes the JIT, which is the optimizer that the JVM uses) can change the code around so that the code in the Helper constructor occurs after the write to the helper variable. If it does this, then after the constructing thread writes to helper, but before it actually finishes constructing the object, another thread can come along and read helper before it is finished initializing.
See the double-checked locking is broken declaration for a much more detailed explanation.
Anyway, the question I always get is "does making the helper field volatile fix this?" The answer is yes. If you make the helper field volatile, the actions that happen before the write to helper in the code must, when the program executes, actually happen before the write to helper — no sneaky reordering is allowed.
Having said this, there are often much better ways of lazily initializing a singleton. One of the items in Josh Bloch's new revision of Effective Java (lgt amazon)
deals exclusively with this problem. This book is chock-full of useful Java knowledge, and is highly recommended for all Java programmers. He has revised it for Java 6; if you only have the old edition, pick this up, because he deals with things like generics, the new threading stuff in JDK5, autoboxing, and how these things all work together to make your life unpleasant.
Posts
50 comments:
Sorry for first bad post and for my server being down, anyway thats exactly what I needed to hear. Are you suggesting approach of nested class by Joshua Bloch with init on demand?:
public class Singleton {
protected Singleton() {
}
private static class SingletonHolder {
static final Singleton instance = new Singleton();
}
public static Singleton getInstance() {
return SingletonHolder.instance;
}
}
Im not sure about keyword modificators but will this run thread safe by jvm itself or does it need additional synchronization?
Thanks
Sorry for first bad post and for my server being down, anyway thats exactly what I needed to hear. Are you suggesting approach of nested class by Joshua Bloch with init on demand?
That is thread safe, and is in the first edition of Effective Java, but the new edition, which was released a couple of weeks ago, has much, much more on this topic. It is definitely worth considering buying it.
Sure i understand you cant say it out loud and i definitelly am interested in this stuff, but i have to wait till they ship it locally in Europe first.
Sure i understand you cant say it out loud and i definitelly am interested in this stuff, but i have to wait till they ship it locally in Europe first.
Oh, I meant to answer your question with "this is thread safe". The Initialization on Demand Holder idiom is a terrific way to initialize static singletons lazily.
I write codes to verify it, but I find it works well in JDK6.0. Why do you say that it is wrong?
public class DoubleLockCheck {
private Object lock = null;
public Object getLock() {
if (lock == null) {
synchronized (this) {
if (lock == null) {
lock = new Object();
}
}
}
return lock;
}
public static void main(String[] args) {
final DoubleLockCheck check = new DoubleLockCheck();
final TreeSet<String> set = new TreeSet<String>();
for (int i = 0; i < 150; i++) {
new Thread(new Runnable() {
public void run() {
Object lock = check.getLock();
set.add(lock.toString());
}
}).start();
}
System.out.println("////////////////////////////////////////////////////////");
System.out.println("allocated lock number is : " + set.size());
for (String str : set) {
System.out.println(str);
}
}
}
Hi Jeremy, I want to ask you about Joshua Bloch's Effective Java. If you happened to read both first and second edition, is second one upgrade with all from first one or is second one remade and missing some parts from first one. Or is it any relevant to ask this? Thanks
Marek, take a look at http://smallwig.blogspot.com/2008/04/i-get-to-break-awesome-news.html
Specifically, here's some of the text:
You probably all know how valuable the first edition is already. The new edition really takes it a step further. It's vastly improved and has entire new sections on generics, enums, annotations, and other recent Java developments. The concurrency chapter was completely redone to reflect the "java.util.concurrent" new world order. There's a wealth of new information about serialization pitfalls and patterns, and the list goes on.
It is not just the Effective Java you know with a few extra chapters tacked on! Josh has painstakingly revisited every single line of every single page. I believe it shows.
This book will certainly replace its predecessor as the bible of our craft. Many of the code reviews I do for Java library code at Google basically end up with me spouting chapter and verse from EJ, and I can't wait for everyone to get the new edition so I can start doing the same with it!
I write codes to verify it, but I find it works well in JDK6.0. Why do you say that it is wrong?
The broken-ness I've described is a function of what the compiler might do, not what the compiler does do. Unfortunately, it is very difficult to predict when a compiler might do something. And, when it does, it is also very difficult to predict when the scheduler will trigger the correctness bug.
As a result of this, you might get 999,999 runs where the right thing happens, but on the 1,000,000th, the wrong thing might happen. Or, you might switch JVMs and trigger the wrong behavior. This is the kind of thing where it is better to catch the error up front, especially since all you have to do is add the word "volatile".
Thanks Robert, and sorry Jeremy to spam your blog a little ;) Cant wait to see it in EU stores, seems to be the javists' must-read print.
Thanks Robert, and sorry Jeremy to spam your blog a little ;)
No problem :) I invited it by suggesting everyone read EJ2.
I'd answer your question, but Robert's already done that. Josh has gone through and updated every line, as well as finally updating it for JDK5 and 6 features. Well worth the cost.
Going on the same lines as above, is this code thread safe:
Say I have an instance variable
List foo = new ArrayList();
Are the below operations thread safe?
foo = new ArrayList();
request.setAttribute("foo", foo);
i.e Is there a probability that the request object will ever have a semi initialized List (like the helper object in the example you gave)?
@abhirama -- where is the other thread?
Sorry for not being clear.
What I meant by the other thread was, say the block of code
foo = new ArrayList();
request.setAttribute("foo", foo);
is concurrently accessed by multiple threads. Then is there a probability that the request object will ever have a semi initialized List (like the helper object in the example you gave)?
I am fine if the foo object in the request is not the latest one but what I am curious about is, is there a probability that the request object can ever have a semi initialized collection?
Hi Jeremy
Why does the Initialization on Demand Holder idiom work properly? Consider the code
protected Singleton() {
}
private static class SingletonHolder {
static final Singleton instance = new Singleton();
}
public static Singleton getInstance() {
return SingletonHolder.instance;
}
}
Is it because, when JVM loads a static class SingletonHolder , the class initialization itself is assuredly locked by JVM? There is no way that "instance" variable can be published before the Singleton() object is constructed? Am I right?
Vignesh
I think my question has been answered here
http://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom
Vignesh
Hi Jemery. In his Effective Java 2 on the page 282 Joshua Bloch has an idiom of lazy initialization with synchronized accessor. With your example it would look like the following:
class Foo {
private Helper helper = null;
public synchronized Helper getHelper(){
if (helper == null) {
helper = new Helper();
}
return helper;
}
}
Note, that helper field is not volatile as in the example in the book. I always though that the semantic of synchronized method is the same as if surround everything in a method with synchronized(this) { }. If yes, then why does the field must be volatile in double-checked locking idiom, when the only difference between it and synchronized accessor is a check for null? If no, then perhaps all the code written inside synchronized(this) {} must modify volatile variables only for it to work correctly.
HI serega,
Note, that helper field is not volatile as in the example in the book. I always though that the semantic of synchronized method is the same as if surround everything in a method with synchronized(this) { }. If yes, then why does the field must be volatile in double-checked locking idiom, when the only difference between it and synchronized accessor is a check for null? If no, then perhaps all the code written inside synchronized(this) {} must modify volatile variables only for it to work correctly.
Don't worry -- the code written inside the synchronized block doesn't have to modify volatile fields if the fields are never being read outside the synchronized block.
The simple version of the reason double-checked locking doesn't work without volatile is that when the helper field is read, no explicit communication happens between threads. With a volatile read, you are telling the system that the update to the field might have happened in a different thread, so it should make sure it does what needs to be done when reading a value written by another thread.
If you leave out the volatile modifier here, the system won't do what needs to be done to read a value written by another thread, so you might end up with the wrong value when you read the contents of the object.
I hope that helps! I know this stuff can be confusing.
@abhirama
I am fine if the foo object in the request is not the latest one but what I am curious about is, is there a probability that the request object can ever have a semi initialized collection?
Sorry I didn't notice your post before. That code is very broken. ArrayList is not designed to work with multiple threads.
Say we replace the ArrayList with a Vector. Then?
@abhirama -- Changing it to a Vector doesn't make it completely correct, because there still is no synchronization between the two threads. However, in the Vector case, there is a possibility that he resulting code would be correct. I would have to look at the rest of the code to know whether it actually worked or not for its intended purpose.
Hey Jeremy,
Sorry to drag this. I understand the issues involved with synchronization and all but what I am really curious about is as to whether there is a probability that my request object will have a semi initialized collection in the scenario I described?
@abhirama: Without seeing all the code, it is hard to say, but given how you described it, it is likely that it would end up with corrupted / incorrect data (at least, according to the memory model).
/* can change the code around so that the code in the Helper constructor occurs after the write to the helper variable */
By this, do you mean that this line: helper = new Helper() may become something like these (scheudo-code)?
helper = obj; // helper var is assigned
obj.ctor(); // the construction is actually called
It's great if you can elaborate on the exact code that are produced by the compiler in such rare cases.
Thanks.
helper = new Helper()
may become something like these (scheudo-code)?
helper = obj; // helper var is assigned
obj.ctor(); // the construction is actually called
It's great if you can elaborate on the exact code that are produced by the compiler in such rare cases.
Thanks.
I'll do some pseudo-code. Let's say the Helper class looks like this:
class Helper {
int x;
Helper() {
x = 1;
}
}
Let's say we have:
helper = new Helper();
this is effectively the same as:
local = <malloc Helper object>;
local.<init>();
helper = local;
Frequently, method invocations can be inlined:
local = <malloc Helper object>
local.x = 1;
helper = local;
Ah, but perhaps the compiler doesn't want to spill a register, so the compiler does this instead:
helper = <malloc Helper object>
helper.x = 1;
Then another thread can sneak in and read helper.x before it is set.
I hope that helps.
That's clear. Thanks a lot, Jeremy. I am interested in learning about concurrency and have found your blog very useful.
Buu
Thanks Jeremy.
Does this mean that assignment operation in Java (involving instance objects) is not thread safe?
Like
foo = new Foo();
Assume foo is an instance variable.
@abhirama -- a variable assignment in isolation is nether thread safe nor thread unsafe. However, this code:
class MyObject {
int x = 1;
}
Thread 1:
MyObject o = new MyObject();
Thread 2:
if (o != null) {
System.out.println(o.x);
}
Should not be assumed to print 1.
What I meant was
//instance variable
List foo;
Thread1:
foo = new Vector();
request.setAttribute("foo", foo);
Thread2:
List foo = (List) request.getAttribute("foo");
//some list iteration code
Above is not thread safe (because of the list initialization code) because there might be a chance that foo in thread 2 retrieved from the request object might be in a semi initialized state. Or am I wrong :)?
BTW thanks for the wonderful blog. Have learnt a lot from it.
@abhirama -- that is correct. There is no synchronization in the Vector constructor, so you can't pass it from one thread to another without additional synchronization.
Hi Jeremy,
With regarding to abhirama's example:
It seems to me that when the setAttribute("foo", foo) is called by Thread 1, the foo object has been properly initialized because there's a happens-before there ("Each action in a thread happens-before every action in that thread that comes later in the program order.").
Therefore, when Thread 2 invokes request.getAttribute("foo"), it should have a fully initialized foo.
Therefore, I don't understand your answer. (Or I may misunderstand the happen-before statement.) Please help explain.
Thanks,
Buu
@buu - There is a happens-before order between the initialization of the vector and the call to setAttribute -- they are in the same thread. There is no happens-before order between the call to setAttribute and the call to getAttribute.
This mean that there is nothing explicitly communicating the initialization of the vector to the second thread. See my post on volatile variables as to why this is wrong. Whatever setAttribute happens to do might get "leaked out" to the other thread, but that doesn't mean that the other thread will necessarily see the correctly constructed vector.
Of course, this is unlikely to bite you in practice.
Got that. Thanks, Jeremy!
Hi Jeremy,
I'm a bit confused whether the "Double Checked Locking" works under the new (JK 5) memory model or not. You claim it does, while this page
http://www.ibm.com/developerworks/java/library/j-dcl.html
says it does not (I'm posting here, because you have comments and answer them!).
I appreciate your help,
Jens
@jens - This is a pre-1.5 article, and the update is less clear than it should be. Double-checked locking works if you declare the field volatile.
Hi,
I have another quizz for you. It's what I'll called the "doesn't need to been instantiated once" singleton pattern.
Here's an example:
private static Method method;
private static void initialize() throws Exception {
if(method == null) {
Method temp = SomeClass.class.getDeclaredMethod("someMethod", new Class[0]);
temp.setAccessible(true);
method = temp;
}
}
Nothing is synchronized because I don't care if at the beginning, two threads get the method. As some point the method will be set and seen by all thread anyway.
That's just one thing I want to be sure. It's that setAccessible is called before method is used.
If method is not volatile, can it be reordered to be set before setAccessible?
Which just made me thought... Is there a performance cost in reading a volatile variable that is never written? (but then, I'm not quite sure of the volatile definition for Java 1.3 and 1.4 (which I also have to support)
@Henri - There isn't anything in the spec about synchronization / visibility rules for this code. It is likely to work, but you never know.
The performance cost of reading a volatile depends on the platform. On x86, it is negligible.
Thanks a lot for the answer. I guess I'll stick with synchronized. Safer :-)
Hi Jeremy,
I have a question on using the primitive datatype to overcome DCL issue. Do you think the below code will solve the problem ? (without the volatile keyword). Please give your thoughts.
class Foo {
private Helper helper = null;
private boolean bHelper = false;
public Helper getHelper() {
if (!bHelper) {
synchronized(this) {
if (!bHelper) {
helper = new Helper();
bHelper = true;
}
}
}
return helper;
}
}
Thanks,
Ebith
@Anonymous: I'm afraid that doesn't work. The thing to understand is that there is nothing special about references or primitive values - the same reorderings can take place.
Hi Jeremy! Is it true that if Helper is immutable then we don't have to make helper field volatile?
@Dmitry - Hi, and thanks for your question! If the helper field is final, then you can't really set it lazily. If you set it in the constructor, and it is final, then you don't need to mark it volatile. I have a relatively long series explaining what final guarantees are, starting here:
http://jeremymanson.blogspot.com/2008/04/immutability-in-java.html
I read you blog with great attention and we have a similar situation where our application crashes sometimes for unknown reasons. I found out that we are doing the following in our code.My question is Class.newInstance() same as creating "new" keyword.
private MyFactory getFactory(){
if (bFactory == null) {
synchronized (this) {
if (bFactory == null) {
try {
String className = "xxxx";
Class MyFactoryClass = null;
MyFactoryClass = Class.forName(className);
bFactory = (MyFactory) MyFactoryClass.newInstance();
}
}
return bFactory ;
}
Class.newInstance is certainly the same. I have no idea if that is what is causing your crash, of course.
Thanks for the reply. I have another question, if the second thread gets an uninstantiated object while the first thread is in process of instantiating it, then doesn't it mean that second thread has an object that is not fully initialized and this will break the code as any call on that uninitialized object will not going to work?
@Anonymous - pretty much. You can call the methods, but they may or may not see the correctly instantiated fields.
Once again thanks alot for your reply. I have one more question on the synchronization section of the code you posted. What will happen if I change synchornized(this) to synchronized (Foo.class)? Does it behaves differently and can it be of any help in solving the double check issue?, kindly explain.
// Broken -- Do Not Use!
class Foo {
private Helper helper = null;
public Helper getHelper() {
if (helper == null) {
synchronized(Foo.class) {
if (helper == null) {
helper = new Helper();
}
}
}
return helper;
}
@Anonymous - it doesn't make any difference. In general, it is probably a bad idea to synchronize on the class object, because you never know who else might be synchronizing on it.
I have a general question on the thread safety and this is not directly related with your blog. I would appreciate if you could post it on your blog. I have a class that has only one static method that accepts two string parameters and does some operation locally (as shown below). Do I need to synchronized this method?
public class A {
public static boolean getDate(String str, String str2){
SimpleDateFormat formatter= new
SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
boolean isBefore = false;
Date date1;
Date date2;
try {
date1 = formatter.parse(str);
date2 = formatter.parse(str);
isBefore = date1.after(date2);
} catch (Exception e) {
e.getMessage();
}
return isBefore;
}
}
@aa - Your wish is my command.
Post a Comment