4

I am new to Java and the following might be obvious, but it is puzzling to me. Consider the following code:

while(1>0){
  if(x!=0){
  //do something
  }
}

The x variable is changed in a different thread. However, the code in the if statement is never executed even when x is not zero. If I change the code by the following

while(1>0){
  System.out.println("here");

  if(x!=0){
  //do something
  }
}

the code in the if statement is now executed when x is no longer zero. I suspect this has to do with the rules of the Java compiler, but it is very confusing to me. Any help clarifying this would be greatly appreciated.

Ivan
  • 409
  • 4
  • 17
  • `x` is int? or any wrapper object? – Vaandu Dec 21 '11 at 07:33
  • 2
    just an FYI and you might know this already: you can do `while(true)` if you want a `while` loop to always run. – Joel Dec 21 '11 at 07:34
  • x is char and belongs to an object X. In turn, X is a static member of a class. I realize it is not the best way to do this for my purposes, but I was surprised at the results. – Ivan Dec 21 '11 at 07:38
  • It's definitively not compiler's guilt. But JIT optimization can influence on time available to peek the x update from different thread. Look at http://wikis.sun.com/display/HotSpotInternals/PerformanceTechniques – Vadzim Dec 21 '11 at 07:52
  • The one thing I don't understand though is that even after the thread where the x value has been changed is completed, the if statement is still not executed. And also, somehow having System.out.println the compiler decides not to optimize? – Ivan Dec 21 '11 at 07:58

5 Answers5

7

If x is changing in a different thread then you are probably seeing a side-effect of the fact that you have not synchronized access to that variable.

The Java memory and threading model is pretty complex, so I'd recommend you get a copy of Java Concurrency in Practice by Brain Goetz and have a read.

The short answer is to make sure that access to x is enclosed in a synchronized block:

while (1 > 0) {
    int temp;
    synchronized (this) {
        temp = x;
    }
    if (temp != 0) {
        // Do something
    }
}

And similarly in the code that modifies x.

Note that this example stores x in a temporary variable, because you want synchronized blocks to be as small as possible - they enforce mutual exclusion locks so you don't want to do too much in there.

Alternatively, you could just declare x to be volatile, which will probably be sufficient for your use case. I'd suggest you go with the synchronized version because you'll eventually need to know how to use synchronized properly, so you might as well learn it now.

Cameron Skinner
  • 51,692
  • 2
  • 65
  • 86
  • Thank you for your answer. Could you also explain why the second code I had worked without the need for either volatile or synchronized? – Ivan Dec 21 '11 at 07:49
  • I don't know why it worked with the second version. Synchronization bugs are subtle and difficult to diagnose for exactly this reason: sometimes things work, sometimes they don't. You often won't discover a synchronization bug until your code has been running for some time. It could be that the fact that `System.out.println` is very slow meant that the JVM had an opportunity to update the thread's memory model, but that's pure speculation. – Cameron Skinner Dec 21 '11 at 08:27
2

If you are using multithreading code, check that x variable is volatile.

dbf
  • 6,399
  • 2
  • 38
  • 65
  • You are right, declaring x as volatile makes the first code execute correctly. But, why is the second code not affected by how x is declared? – Ivan Dec 21 '11 at 07:44
1

Th reason nothing happens without the System.out.println("here"); is well explained by Cameron Skinner's answer.

So why does the block inside the if(x!=0) works when println is used? println executes a synchronized block (see PrintStream.write(String s)). This forces the current thread to retrieve System.out state from the main memory and update the thread's cache, before letting the thread to execute any further line of code. The surprising side effect, is that also states of other variables, such as your x, are also updated in this manner, although x's lock was not involved in the synchronization. It's called piggybacking.

If I'll use free text to describe the formalities described in the Java Memory Model Specification: it is said that operations executed before a release of a lock happen-before operations executed after the next obtaining of that lock.

I'll demonstrate it with an example. Assume that Thread 1 is executed and only when it finishes, Thread 2 is started. Also assume that x, y and z are variables shared by both threads. Note that we can determine z's value only inside the synchronized block of y.

Thread 1:
x = 0;
synchronized(y) {
}

Thread 2:
x = 1
z = x;
  // here there's no guarantee as to z value, could be 0 or 1
synchronized(y) {
    z = x;
      // here z has to be 0!
}

This is of course a very bad practice to rely on for synchronization...

yair
  • 8,945
  • 4
  • 31
  • 50
  • Thank you, this made a lot of sense! – Ivan Dec 21 '11 at 10:09
  • One more question though, why isn't the if statement accessed even after the thread that modifies x is done? – Ivan Dec 21 '11 at 10:21
  • The JMM spec implies no guarantee as of the order of operations that occur in two different threads, upon completion of a thread. But it does guarantee the order upon synchronization. – yair Dec 21 '11 at 13:20
0

This is likely a compiler optimization. It recognizes that, within the scope of your while loop, the variable never changes and will cache the value rather than reading it from memory each time. In order to avoid this behavior, simple declare the variable as volatile:

private volatile int x;
Matthew W
  • 197
  • 1
  • 8
0

Check also what you are doing inside if clause. It they are only memory operations it is a hell lot faster than writing to console, so you are executing much more operations in last case before changing state.

Fernando Miguélez
  • 11,196
  • 6
  • 36
  • 54