Friday, May 23, 2008

Double Checked Locking

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.

101 comments:

Zeratul said...
This comment has been removed by the author.
Zeratul said...

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

Jeremy Manson said...

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.

Zeratul said...

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.

Jeremy Manson said...

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.

Anonymous said...

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);
}
}
}

Marek said...

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

Robert Konigsberg said...

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!

Jeremy Manson said...

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".

Marek said...

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.

Jeremy Manson said...

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.

abhirama said...

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)?

Jeremy Manson said...

@abhirama -- where is the other thread?

abhirama said...

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?

Anonymous said...

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

Anonymous said...

I think my question has been answered here

http://en.wikipedia.org/wiki/Initialization_on_demand_holder_idiom

Vignesh

serega said...

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.

Jeremy Manson said...

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.

Jeremy Manson said...

@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.

abhirama said...

Say we replace the ArrayList with a Vector. Then?

Jeremy Manson said...

@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.

abhirama said...

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?

Jeremy Manson said...

@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).

Buu said...

/* 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.

Jeremy Manson said...


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 said...

That's clear. Thanks a lot, Jeremy. I am interested in learning about concurrency and have found your blog very useful.

Buu

abhirama said...

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.

Jeremy Manson said...

@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.

abhirama said...

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.

Jeremy Manson said...

@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.

Buu said...

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

Jeremy Manson said...

@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.

Buu said...

Got that. Thanks, Jeremy!

Jens said...

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

Jeremy Manson said...

@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.

Henri said...

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)

Jeremy Manson said...

@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.

Henri said...

Thanks a lot for the answer. I guess I'll stick with synchronized. Safer :-)

Anonymous said...

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

Jeremy Manson said...

@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.

Dmitry said...

Hi Jeremy! Is it true that if Helper is immutable then we don't have to make helper field volatile?

Jeremy Manson said...

@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

Anonymous said...

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 ;
}

Jeremy Manson said...

Class.newInstance is certainly the same. I have no idea if that is what is causing your crash, of course.

Anonymous said...

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?

Jeremy Manson said...

@Anonymous - pretty much. You can call the methods, but they may or may not see the correctly instantiated fields.

Anonymous said...

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;
}

Jeremy Manson said...

@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.

aa said...

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;
}
}

Jeremy Manson said...

@aa - Your wish is my command.

Jawad said...

@aa i believe you do not need synchronization since there is no shared resource access.

Anonymous said...

Hi Jeremy - Loved reading your post and responses. I'm wondering if you might consider the code below and comment why it's broke, this solution is just too obvious not to be broken! It doesn't appear to suffer from the problem of "Helper constructor occurs after the write to the helper variable" since it's using a boolean primitive in the if statements.

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;
}

Jeremy Manson said...
This comment has been removed by the author.
Jeremy Manson said...

@anonymous - that doesn't work. The trick is to remember that the guarded code is basically doing 4 things:

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.

Jon Tasco said...

Thanks for the response, Jeremy. If I understand the volatile keyword correctly, wouldn't making the initialized instance variable volatile fix the issue? ...by ensuring it won't be re-ordered like you described?

Jeremy Manson said...

@Jon - Yes; that's more-or-less the same as making the helper reference volatile.

Jon Tasco said...

Greatly appreciated! Thanks again, Jeremy.

Anonymous said...

In that last example, assuming threads A and B enter getHelper. Can't thread A get true for...

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?

Anonymous said...

Regarding my last comment, I guess the main point of double checked locking isn't to avoid two threads competing for a lock and entering the sync block, rather it's to avoid initializing more than once. If I understand correctly, what i said is possible, but only one thread will perform the init stuff, and the second will simply obtain a lock, get false on the second if, and release the lock.

Jeremy Manson said...

@Anonymous - Your interpretation is correct.

Anonymous said...

about the last example,

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
}
}

Jeremy Manson said...

@Anonymous - That doesn't really change anything.

satish said...

I read that Each thread maintains its own local copy (cache..?) of instance variables and when ever we do the modifications, local copy gets changed and these changes are flushed to main memory( just before lock releases). is that true ?

Jeremy Manson said...

@satish - No, that isn't true. It sounds as if you might have been reading about the older memory model, which has been superceded.

Anonymous said...

It seems there are two possible problems: double-instantiating the 'singleton' and accessing a non-initialized 'singleton'.
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?

Anonymous said...

... never mind; i see now that the problem would be reordering of _inlined_ constructor with the volatile write, and that is now disallowed.

Anonymous said...

Sorry I'm not more coherent, but another thought occurs:
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?

Jeremy Manson said...

Hotspot is pretty careful about constructors, because of constraints with final fields. But it certainly can inline them, and I'm not sure why you think any particular example is more complicated than any other.

Anonymous said...

Jeremy,
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.

Jeremy Manson said...

@Anonymous - you posted on the wrong blog entry. You meant to post here:

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.

mesut said...

If we declare helper as static, does it make any difference?

And thanks for doing great job. I think all java developer should read your blog.

Jeremy Manson said...

@mesut - Thanks for the kind words. I assume that you mean that you should declare both helper and getHelper as static. That doesn't make a difference about the correct semantics, although for static lazy initialization, it is worth considering the Initialization On Demand Holder idiom (see elsewhere in the comments, somewhere).

Anonymous said...

..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..

Hi, what part of JLS states that Helper constructor finishes before the assignment to helper variable? Expression evaluation order?

Thanks.

JSparrow said...

"..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.."

Hi, from what part of JLS follows that Helper constructor must be executed before assignment to helper variable?

Thanks.

Jeremy Manson said...

@Anonymous - It is guaranteed because of the semantics of volatile. See my post on volatile:

http://jeremymanson.blogspot.com/2008/11/what-volatile-means-in-java.html

JSparrow said...

Hi. thanks for the link, but I still don't understand. JLS states: "A write to a volatile variable v synchronizes-with all subsequent reads of v" and "If an action x synchronizes-with a following action y, then we also have hb(x, y)" So when Thread 1 writes reference to Helper object to volatile variable (helper) then all subsequent reads of other Threads of helper variable indeed are guaranteed to see not null but reference to the object. But this doesn't say anything about the referenced object state. I'd like to get a reference to the JLS topic that states that an object constructor last statement happens-before the assignment in "helper = new Helper()" like expression.

Thanks.

Jeremy Manson said...

It's the first bullet point here:

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.

JSparrow said...

but what JLS rule guarantees this program order (construct;assign)?

"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.

Jeremy Manson said...

Informally, all that it means to enforce program order between the end of the constructor and the assignment to helper is that those two things are ordered in the source code.

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

JSparrow said...

:) That's I thought. I wrote it in my first comment "..Hi, what part of JLS states that Helper constructor finishes before the assignment to helper variable? Expression evaluation order?.."

Thanks a lot for clarification.

shanmukhan said...

Hi,
please let me know if double checked locking works in java 6 without volatile.

Thanks
Shanmukhan

Jeremy Manson said...

shanmukhan : Why do you think it would? As usual, if it works, it works by accident.

Unknown said...

Lets say I have a class, that uses "On Demand holder idiom":

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!

Jeremy Manson said...

@Unknown - not enough information in your post. If the object itself is immutable, and the database is a SQL database (or something like that), then you should be fine. But if the getters make modifications to fields or array values, you should synchronize them, or rely on modifications that occur after you have finished constructing the object, you should use synchronization.

Unknown said...

Thanks for the reply.

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.

Anonymous said...

I'm sorry to bring this up again, but this intellectual curiosity is beyond me: Already many people have proposed the (broken) fix to the broken DCL idiom that uses a boolean primitive field to save the day. You state in one reply (correctly) that the reason is the possible reorderings that may occur within the synchronized block.

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

Jeremy Manson said...

@Unknown: It's your use of the term "global variables" that throws me. "Global variable" is not a Java concept. Having said that, if your methods do not affect shared data, then you don't need synchronization.

Jeremy Manson said...

@Giannis: What's wrong with that is that the compiler can determine that _helper will never be null at that point, remove the if guard, and do the reordering as per the original example.

Anonymous said...

Dear Jeremy,

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

Anonymous said...

I'm confused...can't another thread modify the Helper instance in between the first "if (helper == null)" call and the "synchronized(this)" call? It seems like you HAVE to do the synchronization before you can check the state...

Jeremy Manson said...

@anonymous: Yes, another thread can grab the lock, update it, and then release the lock. Then, the first thread will grab the lock, see that it has already been constructed, and not re-construct it. Subsequent threads will read helper, see a non-null value, and not have to grab the lock.

Again, this only works if helper is volatile, per the article.

Christian said...

Hi!

I thought the synchronized block would establish a happens-before relationship. If that's the case, why can the JVM reorder the statements?

Jeremy Manson said...

@Christian: both the writer and the reader need a synchronized block (or volatile access). In the original case, the writer has one, but the reader doesn't.

In terms of reordering, the system is unlikely to reorder *around* the synchronized block, but note that it isn't doing that.

vikas singh said...

Hi jeremy,

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?

vikas singh said...

Hi jeremy ,

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

Jeremy Manson said...

@Vikas: the commenter Stephen C who followed up to your question is correct.

Sankar Banerjee said...

One question jeremy. I was going through the specification in the following URL.

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!

Jeremy Manson said...

@Sankar - that's completely off-topic of this post. I recommend you contact a reputable mailing list for these questions, like concurrency-interest:

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.

Michaël REMOND said...

Hello Jeremy,

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

Jeremy Manson said...

Michaël: That article not only predates Java 5, but also predates Java 4.

Michaël REMOND said...

Thank you very much for your response. I understand now all improvements made on the JMM.