While loop not ending when flag changed in different thread

Question

I have a while loop running in my Java program's main method. The loop is supposed to run until a boolean flag variable is set to true in the program's keyPressed method (I added the program as a KeyListener to a JFrame).

import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import javax.swing.JFrame;

public class ThreadWhile implements KeyListener {
    private boolean flag = false;

    public static void main(String[] args) {
        //Set up key listening on a dummy JFrame
        JFrame frame = new JFrame("Key Detector 9000");
        frame.setVisible(true);
        ThreadWhile tw = new ThreadWhile();
        frame.addKeyListener(tw);

        System.out.println("Looping until flag becomes true...");
        while(!tw.flag) {
            //Commenting the println out makes the loop run forever!
            System.out.println(tw.flag); 
        }
        System.out.println("Flag is true, so loop terminated.");
        System.exit(0);
    }

    public void keyPressed(KeyEvent e) {
        flag = true;
        System.out.println("flag: " + flag);
    }

    public void keyReleased(KeyEvent e) {}
    public void keyTyped(KeyEvent e) {}
}

It is my understanding that keyPressed methods execute in their own threads, so it seems like when I hit a key, the variable 'flag' should be set to true, and the while loop running in the main method should end.

HOWEVER, when I run this program, the loop runs forever, even though we can see that the 'flag' variable is being set to true correctly! Oddly, the program behaves correctly if I insert a quick System.out.println echo of the 'flag' variable inside the while loop, but obviously I don't want to print anything out in the loop.

I'm guessing this problem might be a result of the Java compiler trying to optimize the empty while loop to the point where it stops actually checking the 'flag' variable? Does anyone have suggestions for making this work correctly, or perhaps some more nifty concurrency-based approaches to making the main thread pause until the keyPressed thread executes?

Thanks!

Use guarded blocks instead (http://docs.oracle.com/javase/tutorial/essential/concurrency/guardmeth.html). Looping and expecting something to become true will ruin the performance of your application and make it unresponsive. — Chris Dennett, Jul 21 '12 at 21:18

score 10 · Accepted Answer · answered Jul 21 '12 at 21:14

10

You need to declare the flag volatile, otherwise the compiler can optimize your code and skip the reads of the flag.

answered Jul 21 '12 at 21:14

Sergey Kalinichenko

714,442
84
1,110
1,523

Turix · Answer 2 · 2012-07-21T21:38:25.957

While the volatile solution that others proposed should work, unless you need the code in the while-loop to execute continuously, you should probably instead use wait() and notify() (or notifyAll()) inside of a synchronized section (to avoid "busy waiting"). Something like:

public class ThreadWhile implements KeyListener {
    private boolean flag = false;
    private Object flagLock = new Object();

    public static void main(String[] args) {
        //Set up key listening on a dummy JFrame
        JFrame frame = new JFrame("Key Detector 9000");
        frame.setVisible(true);
        ThreadWhile tw = new ThreadWhile();
        frame.addKeyListener(tw);

        System.out.println("Waiting until flag becomes true...");
        synchronized (tw.flagLock) {
            while(!tw.flag)
                tw.flagLock.wait();   // Note: this suspends the thread until notification, so no "busy waiting"
        }
        System.out.println("Flag is true, so loop terminated.");
        System.exit(0);
    }

    public void keyPressed(KeyEvent e) {
        synchronized (flagLock) {
            flag = true;
            System.out.println("flag: " + flag);
            flagLock.notifyAll();
        }
    }

    ...

Otherwise, you are repeatedly wasting cycles on the main thread checking the value of flag over and over (which, in general, if it happens enough, can slow down the CPU for other threads and may wear out the battery on mobile devices). This is exactly the sort of situation that wait() was designed for.

score 0 · Answer 3 · answered Jul 21 '12 at 21:32

1. You need to use volatile keyword for the flag, Because when we call volatile on an Instance variable, then its like telling the JVM to make sure that the thread accessing it must reconcile its own copy of this instance variable with the one stored in the memory.

2. Volatile will also help in preventing the caching of the value in the thread.

3. Volatile will make that variable reflects its changed value in one thread to the another, but that does NOT prevent the race condition.

4. So use synchronized keyword on the Atomic statements which is setting the value of the flag.

Eg:

public void keyPressed(KeyEvent e) {
     synchronized(this){
        flag = true;
        System.out.println("flag: " + flag);
      }
    }

While loop not ending when flag changed in different thread

3 Answers3

Linked

Related