Why do I get an infinite while loop even if I change the lock variable?

Question

public class GuardedBlock { private boolean guard = false; private static void threadMessage(String message) { System.out.println(Thread.currentThread().getName() + ": " + message); } public static void main(String[] args) { GuardedBlock guardedBlock = new GuardedBlock(); Thread thread1 = new Thread(new Runnable() { @Override public void run() { try { Thread.sleep(1000); guardedBlock.guard = true; threadMessage("Set guard=true"); } catch (InterruptedException e) { e.printStackTrace(); } } }); Thread thread2 = new Thread(new Runnable() { @Override public void run() { threadMessage("Start waiting"); while (!guardedBlock.guard) { //threadMessage("Still waiting..."); } threadMessage("Finally!"); } }); thread1.start(); thread2.start(); } } 

I studied concurrency with a Java tutorial. Got protected blocks and tried to check it. There is one thing that I cannot understand.

While the loop is infinite, but if you uncomment the threadMessage line, everything works fine. Why?

Answer 1

Short answer

You forgot to declare guard as volatile boolean.

---

If you omit the declaration of your field as volatile , you do not tell the JVM that this field can be seen by several threads that take place in your example.

In such cases, the value of guard will be read only once and will cause an infinite loop. It will be optimized for something like this (without printing):

 if(!guard) { while(true) { } } 

Now why does System.out.println change this behavior? Because writes synchronized, which causes threads to not cache reads.

Here is the code paste of one of the println PrintStream methods used by System.out.println :

 public void println(String x) { synchronized (this) { print(x); newLine(); } } 

and write :

 private void write(String s) { try { synchronized (this) { ensureOpen(); textOut.write(s); textOut.flushBuffer(); charOut.flushBuffer(); if (autoFlush && (s.indexOf('\n') >= 0)) out.flush(); } } catch (InterruptedIOException x) { Thread.currentThread().interrupt(); } catch (IOException x) { trouble = true; } } 

Pay attention to synchronization.

Answer 2

Jean-Francois solution is the right one: you absolutely must have some kind of synchronization when the threads access a shared variable, whether through volatile , synchronized , etc.

I would also add that your while is equal to what is called waiting for a wait - that is, by re-testing the condition in a concurrent setting. The close loop of busy waiting in this code may be processor dependent. At least its impact on system resources will be unpredictable.

You might want to explore a conditional variable approach to working with multiple threads affected by one general condition. Java has many higher-level tools for this in java.util.concurrent , but it's good to know the old lower-level API methods, especially since you work directly with Thread instances.

Each Object has wait() and notifyAll() . Object represents a condition, or at least the monitor associated with it. The wait() method is called in a while , which checks the condition and blocks the calling thread until some other thread calls notifyAll() . Then all waiting threads will be woken up, and they will all compete for blocking and the ability to check the status again. If the condition remains true at this point, all flows will continue.

Here's what your code would look like with this approach:

 public class GuardedBlock { private boolean guard = false; private static void threadMessage(String message) { System.out.println(Thread.currentThread().getName() + ": " + message); } public static void main(String[] args) throws Exception { GuardedBlock guardedBlock = new GuardedBlock(); Thread thread1 = new Thread(new Runnable() { @Override public void run() { try { Thread.sleep(1000); synchronized (guardedBlock) { guardedBlock.guard = true; guardedBlock.notifyAll(); } threadMessage("Set guard=true"); } catch (InterruptedException e) { e.printStackTrace(); } } }); Thread thread2 = new Thread(new Runnable() { @Override public void run() { threadMessage("Start waiting"); while (!guardedBlock.guard) { synchronized (guardedBlock) { try { guardedBlock.wait(); } catch (InterruptedException e) { e.printStackTrace(); } } } threadMessage("Finally!"); } }); thread1.start(); thread2.start(); thread2.join(); System.out.println("Done"); } } 

Please note the following:

• A lock should be executed whenever a condition is read or written (via synchronized .)
• guard condition is tested inside the while , but this loop is blocked during the wait() call. The only reason it is still a while tag is to handle situations where many threads and state change many times. Then, the condition must be re-checked when the thread wakes up, if another thread changes the condition in the tiny gap between waking up and re-locking the lock.
• Waiting threads are notified when the guard condition is set to true (via calls to notifyAll() .)
• The program blocks in the thread2 instance thread2 at the very end so that we do not exit the main thread until all threads (via a join() call.)

If you look at the Object API, you will also see the notify() method. The easiest way is to use notifyAll() , but if you want to understand the difference between the two methods, see this SO post. .

Answer 3

The reason your while loop is infinite is because the condition !guardedBlock.guard always true. This means that guardedBlock.guard = true; set to Thread 1 is not set for Thread 2, and this is because you are not using variable protection like volatile .

Let me copy the need to use volatile in java from wikipedia itself:

In all versions of Java, there is a global reading and writing order for a mutable variable. This means that every thread accessing the volatile field will read the current value before continuing instead of (potentially) using the cached value.

Hope this helps.