Java unable to break out of while loop

Question

I am trying to build a timer in java.

Expected Output:

When you run the program, there's a window with a big "0" in it.

Once the user presses any key from the keyboard, the number increases by 1 every second.

Here's what I have.

    import javax.swing.*;
    import javax.swing.SwingConstants;
    import java.awt.BorderLayout;
    import java.awt.Font;
    import java.awt.event.KeyListener;
    import java.awt.event.KeyEvent;

    public class TimerTest implements KeyListener {
        static final int SCREEN_WIDTH = 960;
        static final int SCREEN_HEIGHT = 540;
        static boolean timerStarted;
        static int keyPressedNum; //amount of times the user pressed any key.

        static JLabel label;
        static int currentTime;

    public static void main(String[] args) {

        JFrame frame = new JFrame("TimerTest");
        JPanel panel = new JPanel();

        label = new JLabel();
        label.setFont(new Font("Arial", Font.PLAIN, 256));
        label.setText("0");
        label.setHorizontalAlignment(SwingConstants.CENTER);

        panel.setLayout(new BorderLayout());
        panel.add(label, BorderLayout.CENTER);

        frame.addKeyListener(new TimerTest());

        frame.getContentPane().add(panel);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setSize(SCREEN_WIDTH, SCREEN_HEIGHT);
        frame.setResizable(false);
        frame.setVisible(true);

        while (true) {
            while (TimerTest.timerStarted == false) {
                if (TimerTest.timerStarted == true) {break;}
            }

            try {Thread.sleep(1000);}
            catch (InterruptedException ex) {ex.printStackTrace();}

            currentTime++;
            label.setText(String.valueOf(currentTime));
        }
    }

    public void keyPressed(KeyEvent e) {
        System.out.println("Keypress indicated.");
        TimerTest.timerStarted = true;
    }

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

When I press any key, the program sets timerStarted as true.

Because timerStarted is true, I was expecting this part:

    while (TimerTest.timerStarted == false) {
            if (TimerTest.timerStarted == true) {break;}
    }

to break out of the loop.

Instead when I run the program and press any key, the command line just prints: Keypress indicated., and it doesn't break out of that loop.

Now here's the weird part:

I tried adding some code to the while block, like this.

  while (TimerTest.timerStarted == false) {
        System.out.println("currently stuck here...");
        //(I simply added a println code)
        if (TimerTest.timerStarted == true) {break;}
  }

Surprisingly when I do this, the program works just like how I wanted. It breaks out of that loop and the timer runs.

This confuses me. Adding a System.out.println(); fixes the issue...?

Why does not having System.out.println() makes me unable to break out of the loop?

Any explanations would be appreciated =)

`TimerTest.timerStarted == true` would logically never fall under `TimerTest.timerStarted == false` (unless something infinitesimally unimaginably precise makes a predictable change between the two `if` executions) — ernest_k, Dec 28 '19 at 08:53
Yes, I know it's pretty much the same thing as not having `TimerTest.timerStarted == true`. However, both cases do not work. — Kcits970, Dec 28 '19 at 08:55
What is the actual/real condition under which you want to break out of the loop? I see nothing in the loop changing the value of `TimerTest.timerStarted`. Is it being changed outside? — ernest_k, Dec 28 '19 at 08:56
I want to break out of the loop when `timerStarted` = true. I used the while loop to "wait" until `timerStarted` is equal to true — Kcits970, Dec 28 '19 at 08:57
IIRC, a `break` will break out of the nearest loop, not all loops. You have a `while` in your `while`, so when you break out of the inner `while`, you should still be in the outer `while`. — domsson, Dec 28 '19 at 08:59
Are you seeing the `"Keypress indicated."` output when you hit a key (supposing I interpret your code correctly)? — ernest_k, Dec 28 '19 at 08:59
@dommson, yes I do acknowledge that. the problem is that I am unable to break out of the inner while loop. — Kcits970, Dec 28 '19 at 09:01
@ernest_k yes, i do see the "Keypress indicated" output in the command prompt. — Kcits970, Dec 28 '19 at 09:01
I've also tried your code, and it works as I understand you expect it to. It is indeed breaking out of the inner loop when I hit the keyboard. However, the outer loop still continues. I could confirm that by adding different `println` outputs at the beginning of each while loop body — ernest_k, Dec 28 '19 at 09:07
There is nothing in the code at this time that would allow the outer loop to stop. — NomadMaker, Dec 28 '19 at 09:20

Atef Hares · Answer 1 · 2019-12-28T09:23:39.003

Your issue is because you are using a non thread-safe boolean variable timerStarted

moreover the while(true) loop is running from main thread but the modification you do on timerStarted is done from another thread this gives unexpected behavior completely depends on the machine that runs the program.

This situation is making a race-condition between changing value of timerStarted and checking for that change

How did I detected this:

Simply I printed the thread id while checking for value changes and while modifying the value

The solution

check these code changes

static AtomicBoolean timerStarted = new AtomicBoolean(false);

and

   while (Scratch.timerStarted.get() == false) {
        System.out.println("Thread.ID: " + Thread.currentThread().getId() + " ----> Scratch.timerStarted: " + Scratch.timerStarted);
        if (Scratch.timerStarted.get() == true) {
            break;
        }
    }

and

 public void keyPressed(KeyEvent e) {
        System.out.println("Keypress indicated.");
        Scratch.timerStarted.set(true);
        System.out.println("Thread.ID: " + Thread.currentThread().getId() + " ----> Scratch.timerStarted: " + Scratch.timerStarted);
    }

here I used AtomicBoolean instead of boolean which is thread safe.

What you need to read about:

Multi-threading in java (check this: https://www.javatpoint.com/multithreading-in-java)
AtmoicBoolean vs boolean (check this: When do I need to use AtomicBoolean in Java?)

score 1 · Accepted Answer · answered Dec 28 '19 at 09:37

The problem is that your code is not thread-safe. Specifically, you are not doing the things that are needed to ensure that reads of timerStarted see the most recent write by the other thread.

There are number of possible solutions, including:

declare timerStarted as volatile, or
read and write timerStarted in a synchronized block, or
use an AtomicBoolean.

These ensure that there is a happens-before relation between a write of the flag by one thread and subsequent read by a second thread. That in turn guarantees that the read sees the value that was written.

"Why does not having System.out.println() makes me unable to break out of the loop?"

Because there is no happens-before relationship; see above.

A more interesting question is:

"Why does it work when you add the println?"

The System.out.println() does some synchronization under the hood. (The stream object is thread-safe, and synchronizes to ensure that its data structures don't get messed up.) This is apparently sufficient to cause writes to timerStarted to be flushed to main memory.

However, this is not guaranteed. The javadocs do not specify the synchronization behavior for the println call. Furthermore, it is not clear that this is sufficient to give a (serendipitous!) happens before between the write and read of timerStarted.

References:

Java unable to break out of while loop

2 Answers2