I still get a lot of questions about whether double-checked locking works in Java, and I should probably post something to clear it up. And I'll plug Josh Bloch's new book, too.
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.
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.
Comments
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
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.
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.
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);
}
}
}
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!
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".
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.
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)?
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?
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
http://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom
Vignesh
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.
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.
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.
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?
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.
Buu
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.
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.
//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.
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
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.
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
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)
The performance cost of reading a volatile depends on the platform. On x86, it is negligible.
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
http://jeremymanson.blogspot.com/2008/04/immutability-in-java.html
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 ;
}
// 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;
}
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;
}
}
class Helper {
private Helper helper = null;
private boolean initialized;
public Helper getHelper() {
if (!initialized) {
synchronized(this) {
if (!initialized) {
helper = new Helper();
initialized=true;
}
}
}
return helper;
}
r1 = ... // create the new Helper object
r1.constructor_call(); // invoke the constructor
helper = r1; // assign the resulting value to helper
initialized = true; // set initialized
There is nothing preventing that from being reordered as follows:
r1 = ... // create the new Helper object
helper = r1; // assign the resulting value to helper
initialized = true; // set initialized
r1.constructor_call(); // invoke the constructor
That's broken in a fairly straightforward way.
if (!initialized) {
...and then be preempted by thread B who would also gets true prior to either entering the sync block? Is my line of thinking wrong there?
if you use
if (!initialized)
part in the actual constructor, does it still might do re-ordering stuff??
public Helper getHelper() {
if (!initialized) {
synchronized(this) {
if (!initialized) {
helper = new Helper();
initialized=true;
}
private Helper(){
if (!initialized){
//stuff
}
}
I understand how adding 'volatile' fixes/avoids the first failure, but if the Helper has non-final fields, what is needed to ensure they are set/visible to a [non-constructing] Thread?
If the danger is from _inlined_ constructor code, and DCL is used because the constructor is expensive and complex, is it likely that such a constructor would be candidate for inlining?
The examples of the problem tend to show simple "this.y=4" constructors. Will JIT/hotspot really promote complex constructors?
You wrote "all of the memory contents
seen by Thread 1, before it wrote to ready,
must be visible to Thread 2, after it reads the value
true for ready."
But is it possible that first thread see "answer =42"
and is going to write to "ready" but at this moment
another thread change "answer" to, for example, "43" so
thread that execute "if(ready)" actually get
stale variable "answer"?
Because there is no Atomicity?
With all respect, possibly i just did't get it.
http://jeremymanson.blogspot.com/2008/11/what-volatile-means-in-java.html
Having said that, there is no third thread in that example. If another thread wrote 43, the second thread could certainly see it.
And thanks for doing great job. I think all java developer should read your blog.
Hi, what part of JLS states that Helper constructor finishes before the assignment to helper variable? Expression evaluation order?
Thanks.
Hi, from what part of JLS follows that Helper constructor must be executed before assignment to helper variable?
Thanks.
http://jeremymanson.blogspot.com/2008/11/what-volatile-means-in-java.html
Thanks.
http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.4.5
It isn't constructor-specific; it applies to anything in program order that happens before the write to helper.
"Among all the inter-thread actions performed by each thread t, the program order of t is a total order that reflects the order in which these actions would be performed according to the intra-thread semantics of t."
How it follows from the above that, when executing helper=new Helper(), calling the constructor after the assignment is not comply "intra-thread semantics of t"?
Sorry for my stupidity, but I want to understand JMM.
More carefully, the ordering of actions in the source code is defined by the rest of the JLS. The order in which assignment statements are evaluated is defined here:
http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.26.1
Thanks a lot for clarification.
please let me know if double checked locking works in java 6 without volatile.
Thanks
Shanmukhan
public TemperatureHelper {
private static class SingletonHolder {
static final Temperature instance = new Temperature();
}
public static Temperature getInstance() {
return SingletonHolder.instance;
}
}
And in this class i have a bunch on public getters to fetch values from a database.
Do I need to have the "getters" synchronized as well?
usage: ArrayList temperatures = TemperatureHelper.getInstance().getForYear(2012);
Nice post by the day!
The class "TemperatureHelper" has no global variables, just methods like:
public synchronized ArrayList getTemperaturesForDay(int year, int month, int day) {
ArrayList temperatures = new ArrayList();
Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("GMT+1"));
cal.set(year, month, day, 0, 0, 0);
for (int i = 0 ; i < 24 ; i++) {
// Get temperature value from database.
temperatures.add(getHourTemperature(cal.getTime()));
cal.add(Calendar.HOUR_OF_DAY, 1);
}
return temperatures;
}
I have several methods like this. But since I have no global
variables do i need synchronization? I guess not, but im a bit confused.
What about this proposed fix to the problem (that I claim must prohibit reorderings from happening):
class Foo {
private Helper _helper = null;
private boolean _initialized = false;
public Helper getHelper() {
if (!_initialized) {
synchronized(this) {
if (!_initialized) {
_helper = new Helper();
if (_helper!=null)
_initialized=true;
}
}
}
return _helper;
}
}
Now, the _initialized field within the synchronized block cannot be set to true until the previous (constructor) finishes its job. Further, even though the _initialized field may remain false for all threads (in their local thread cache), when a thread executes the getHelper() method, if it sees the _initialized field still false, it will have to enter the synchronized block and by so doing it will have to read on the next if (!_initialized) block the correct value for this field. After that, whenever this thread calls getHelper() will never have to enter the synchronized block again, and will use the fully constructed _helper object correctly. What is wrong with this reasoning ? (according to the "DCL idiom is broken declaration, there is definitely something wrong with this "fix" as well, I just can't figure out what's wrong with it).
Many thanks for your consideration!
Giannis Christou
You're right. Even though I can imagine ways to force the compiler not to "outsmart the programmer" (e.g., I can call a=System.currentTimeInMillis() right before starting the _Helper, then get again the time b after the ctor is done, and set the _init field to true only if b-a>=0, or smth similar), BUT still, the idiom is broken: the threads that see _init==true without having performed synchronization, may well use the wrong values for the fields of _helper from their thread-local cache... so the essence here is that each thread must be able to do synchronization the first time it calls the method, and then never again (basically Dimitri's -or what's his name- technique using ThreadLocal storage is doing exactly that).
Thanks for your answer again.
Giannis Christou
Again, this only works if helper is volatile, per the article.
I thought the synchronized block would establish a happens-before relationship. If that's the case, why can the JVM reorder the statements?
In terms of reordering, the system is unlikely to reorder *around* the synchronized block, but note that it isn't doing that.
I have a question. Please share your thoughts over below
Below code snippet is from Effective Java 2nd Edition Double Checked Locking
// Double-check idiom for lazy initialization of instance fields
private volatile FieldType field;
FieldType getField() {
FieldType result = field;
if (result == null) { // First check (no locking)
synchronized(this) {
result = field;
if (result == null)// Second check (with locking)
field = result = computeFieldValue();
}
}
return result;
}
From what i know the main Problem with Double Checked Locking is the reordering inside second check locking so that the other thread might see the values of field/result as set which may be infact still be in executing. To avoid this we make the reference of field as volatile to gurantee visibility and reordering.
But this can be achieved by the below code also
private FieldType field; // non volatile
private volatile boolean fence = false;
FieldType getField() {
if (field == null) { // First check (no locking) // no volatile read
synchronized(this) { // inside synch block no problem of visibilty will latest //value of field
if (field == null)// Second check (with locking)
Object obj = computeFieldValue();
fence = true; // any volatile write will take. this will make sure statements are //not reorder with setting field as non null.
field = (FieldType)obj; // this will be only set after computeFieldValue has been //completed fully
}
}`enter code here`
return field;
}
So after when the initialization have been done, then no thread will have to for volatile read or synchronization overhead. Please see if my assumptions are right or not?
please look at my question..
http://stackoverflow.com/questions/17169145/is-this-a-better-version-of-double-check-locking-without-volatile-and-synchroniz
thanks
vikas
http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html
I did not understand the following line in 17.2.3. Interruptions , which says "Additionally, if there exists some object m whose wait set contains u, then u is removed from m's wait set. This enables u to resume in a wait action, in which case this wait will, after re-locking m's monitor, throw InterruptedException."
If u could explain to me. Thanks in advance!
http://altair.cs.oswego.edu/mailman/listinfo/concurrency-interest
There are also several good books on Java Concurrency, like Brian Goetz's Java Concurrency in Practice.
Anyway, all that statement means is that a thread that is in Object.wait() will no longer be waiting when it gets interrupted, and may wake up, re-acquire the lock it was holding, and throw InterruptedException.
Sorry to post on this old thread again, but the (old) Brian Goetz article (http://www.javaworld.com/article/2074979/java-concurrency/double-checked-locking--clever--but-broken.html) says that unless all Helper fields are declared volatile too, declaring only the helper field as volatile is not sufficient. Maybe Brian said that in a pre-Java 5 environment. Do you agree? If yes can you explain what has changed in the volatile definition?
Thank you very much
It has been quite a while since this double checking problem first surfaced.
was wondering if the following code work...
class Foo {
private static final Object lock = new Object();
private Helper helper = null;
public Helper getHelper() {
if (helper == null) {
synchronized(lock) {
if (helper == null)
helper = new Helper();
}
}
return helper;
}
}
does it mean that it's unsafe to write something like this in an un-synchronized block?
this.student = new Student("John Doe", 40);
because another thread that has access to "this" could see a student reference with non yet initialized fields. Is it correct?
Regards,
Konstantin.
There isn't enough context in your question to answer it. If that field isn't going to be shared between threads, then you are fine. If it is, then you need some additional synchronization to protect it - final fields, volatile fields, locking, something.
Thank you for your blog. Really nice reading!
I heard several times about one trick allowing to have a safe double-checked locking initialization without making initialized variable volatile. Here it is:
class Helper {
int mutable;
final int guard;
Helper(int m, int g) {
mutable = m;
guard = g;
}
}
The trick is usually explained as follows. As a final field is guaranted to be seen initialized, a mutable variable initialized before in a program order is guaranteed to be seen initialized too. But I failed to find a justification for it in JLS chapter 17.
Is such idiom safe or is it just a myth?
In actual implementations, this is likely to be safe, because there will probably be a barrier at the end of the constructor. But it isn't guaranteed.