#1283 DC_DOUBLECHECK is not a bug.

[Claude Martin]

I really don't have much to say about this. It simply isn't a bug. It was in Java 1.4.
But this is 2014 and we use Java 8. I double checking all the time and there's no problem.
Sure, some may still use Java 1.4 but it's their problem. Unless the code compliance level is set to 1.4 it should just ignore it.

[Claude Martin]

It was fixed with JSR 133:
https://jcp.org/en/jsr/detail?id=133

[William Pugh]

Double checked locking is still a bug. In all versions of Java.

You can declare the variable as volatile, and then use double checked locking. Or use any of a number of other strategies for initialize-once synchronization.

[Claude Martin]

You can declare the variable as volatile,

Sure you can. It's not always necessary. But I now understand why it would be rather hard for FB to check the code if it works without volatile. I understand it that it is only needed if you assign the return value of some method to the variable. So it doesn't matter of it as a primitive type. it's often a constructor that is called and the JIT would inline it. So the reference is assigned before the object is initialized. Volatile will prevent that.

So if this really is a bug, will the explanation be updated? The text is now rather misleading. It should state that this works with Java 5+ (thanks to JMM) but should only be used on a variable that is primitive or volatile.
The link to http://www.cs.umd.edu/... doesn't really help when someone wants to fix the bug.
Maybe link to this text:
http://www.javamex.com/tutorials/double_checked_locking_fixing.shtml
Or this:
https://blogs.oracle.com/cwebster/entry/double_check_locking

[William Pugh]

Give me an example of "double-checked locking" that you think is valid and that FindBugs complains about. I've gotten lots of such examples from many people, and in every single case the person was wrong and the code wasn't valid. You might be the first to actually come up with valid code where FindBugs complains about double checked locking.

[Claude Martin]

I just think the text is misleading and out of date. I'd love to find an example, but I simply can't reproduce the bug even with example code that is known to be invalid. How could I find a valid piece of code if I can't even find invalid code? As I understand it this only happens with a combination of inlined code and values being partially committed to main memory.

Now the bug description claims this:

This method may contain an instance of double-checked locking. This idiom is not correct according to the semantics of the Java memory model.

Double-checked locking is correct. But only on member variables that are primitive (except long and double) or volatile.

[William Pugh]

No, double checked locking isn’t correct. You are misinformed.

It isn’t correct in Java 4, 5, 6, 7, 8 or 9.
It isn’t correct in C or C++.

Bill

On Sep 22, 2014, at 7:20 AM, Claude Martin claude_m@users.sf.net wrote:

I just think the text is misleading and out of date. I'd love to find an example, but I simply can't reproduce the bug even with example code that is known to be invalid. How could I find a valid piece of code if I can't even find invalid code? As I understand it this only happens with a combination of inlined code and values being partially committed to main memory.

Now the bug description claims this:

This method may contain an instance of double-checked locking. This idiom is not correct according to the semantics of the Java memory model.
Double-checked locking is correct. But only on member variables that are primitive (except long and double) or volatile.

[bugs:#1283] http://sourceforge.net/p/findbugs/bugs/1283 DC_DOUBLECHECK is not a bug.

Status: closed-rejected
Group: 3.x
Labels: DC_DOUBLECHECK Java 1.4 JMM
Created: Sat Jun 14, 2014 05:38 PM UTC by Claude Martin
Last Updated: Sun Sep 21, 2014 01:22 PM UTC
Owner: William Pugh

I really don't have much to say about this. It simply isn't a bug. It was in Java 1.4.
But this is 2014 and we use Java 8. I double checking all the time and there's no problem.
Sure, some may still use Java 1.4 but it's their problem. Unless the code compliance level is set to 1.4 it should just ignore it.

Sent from sourceforge.net because you indicated interest in https://sourceforge.net/p/findbugs/bugs/1283/ https://sourceforge.net/p/findbugs/bugs/1283
To unsubscribe from further messages, please visit https://sourceforge.net/auth/subscriptions/ https://sourceforge.net/auth/subscriptions

[Claude Martin]

Then why does FB accept DCL on primitive/volatile members?! DCL works in Java 5+. In all my tests it even works without volatile. But tests don't prove anything, and I don't claim it works without volatile.
But the bug description is outdated and misleading. Why cant the description actually explain how to fix the bug? Just add volatile and it's fine.

[Tagir Valeev]

DCL doesn't work without volatile even for primitives. You probably don't understand the Java Memory Model. If you don't specify the volatile keyword, either JIT compiler or CPU is allowed to reorder the instructions in any way which doesn't produce visible behavior changes within the current thread. This doesn't mean that your JVM and your CPU in your test will actually use this right, but your code may break on another JVM or another architecture or under another circumstances. Reordering is very useful for optimization: for example, CPU may wait dozens of cycles till cache line is filled from the main memory to execute the next instruction (main memory access is damn slow). So instead of idling CPU may analyze the following instruction and if they could be executed within currently cached data and don't break the current thread logic, it will execute them in advance. Consider for example the following code:

String str;
boolean init = false;

void init() {
    if(!init) {
        synchronized(this) {
            if(!init) {
                str = "initialized";
                init = true;
            }
        }
    }
}

JIT and CPU are allowed to reorder string assignment and init assignment (despite init is primitive), because this doesn't break the thread logic. So you actually may encounter the case that you executed the init() method, but string is still not assigned. Though it would be really hard to write a test to catch this.

If you specify volatile, then you bind volatile variable reads and writes in different threads with happens-before relation. You explicitly says that everything which occurs in threadA before writing the volatile variable must be visible in threadB after reading the same variable. So now both JIT compiler and CPU cannot reorder the instructions to violate this relation (for CPU it's achieved using memory barrier instruction).

This bug pattern is somewhat similar to "RR: Method ignores results of InputStream.read() (RR_NOT_CHECKED)". You may write dozen of tests which will prove that is.read(bytes) always fills the whole array in your code. But by contract it's allowed not to do this, so your code may break under unusual circumstances (for example, if you provide a named pipe instead of regular file as the input).

[Claude Martin]

ok, so we can agree that DCL is a bad idea. JSR133 states that it is a "dubious coding idiom". I posted this report because I was confused by the bug description and it is still confusing and outdated. Some might think (as I did) that this is just outdated and DCL works in Java 5+.