Clock thread in Java

I have been studying about Java Thread recently. I created this simple clock that will display how many seconds have passed. It looked like this

Here is the code for the application above (including 2 classes: Main and ClockPanel)

public class Main {
    public static void main(String[] args) {
        SwingUtilities.invokeLater(() -> {
            JFrame frame = new JFrame("Clock");
            frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
            frame.setSize(300, 300);
            frame.setLayout(new GridLayout(1, 1));

            ClockPanel clock = new ClockPanel();
            frame.add(clock);

            frame.setVisible(true);
        });
    }
}

public class ClockPanel extends JPanel implements ActionListener {

    private JLabel timeValue;
    private JButton runButton;
    private JButton pauseButton;
    private JButton resetButton;

    private boolean isRunning;
    private int seconds;


    public ClockPanel() {
        timeValue = new JLabel("0");

        setBorder(BorderFactory.createLineBorder(Color.BLACK));
        setAlignmentX(Component.CENTER_ALIGNMENT);
        setLayout(new BoxLayout(this, BoxLayout.Y_AXIS));

        initButtons();

        // create panel to display the clocks
        JPanel timePanel = new JPanel();
        timePanel.setLayout(new BoxLayout(timePanel, BoxLayout.X_AXIS));
        timePanel.setAlignmentX(Component.CENTER_ALIGNMENT);
        JLabel timeLabel = new JLabel("Value");
        timePanel.add(timeLabel);
        timePanel.add(Box.createRigidArea(new Dimension(30, 0)));
        timePanel.add(timeValue);
        add(timePanel);

        add(Box.createRigidArea(new Dimension(0, 20)));

        // create panel to display the buttons
        JPanel buttonPanel = new JPanel();
        buttonPanel.setLayout(new BoxLayout(buttonPanel, BoxLayout.X_AXIS));
        buttonPanel.setAlignmentX(Component.CENTER_ALIGNMENT);
        buttonPanel.add(runButton);
        buttonPanel.add(Box.createRigidArea(new Dimension(30, 0)));
        buttonPanel.add(pauseButton);
        buttonPanel.add(Box.createRigidArea(new Dimension(30, 0)));
        buttonPanel.add(resetButton);
        add(buttonPanel);

        // create thread to run the clock
        Thread clockThread = new Thread(() -> {
            seconds = -1;
            while (true) {
//                System.out.println(); // if this line is removed, the clock will not run
                if (isRunning) {
                    try {
                        seconds++;
                        timeValue.setText(String.valueOf(seconds));
                        Thread.sleep(1000);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        });
        clockThread.start();
    }

    private void initButtons() {
        runButton = new JButton("Run");
        pauseButton = new JButton("Pause");
        resetButton = new JButton("Reset");

        runButton.addActionListener(this);
        pauseButton.addActionListener(this);
        resetButton.addActionListener(this);
    }

    @Override
    public void actionPerformed(ActionEvent e) {
        if (e.getSource() == runButton) {
            isRunning = true;

            runButton.setEnabled(false);
            pauseButton.setEnabled(true);
            resetButton.setEnabled(false);
        } else if (e.getSource() == pauseButton) {
            isRunning = false;

            runButton.setEnabled(true);
            pauseButton.setEnabled(false);
            resetButton.setEnabled(true);
        } else if (e.getSource() == resetButton) {
            isRunning = false;
            timeValue.setText("0");
            seconds = 0;

            runButton.setEnabled(true);
            pauseButton.setEnabled(false);
            resetButton.setEnabled(false);
        }
    }
}

The problem is: if i remove the line System.out.println() (as i commented in the code above), the clock will not run. I have no idea why this is happening at all. Can someone explain it to me? Is it because System.out.println() affect running thread or something?

Answer

isRunning is not volatile and therefore the value each thread sees can diverge. You have a loop that just spins when the value is false and has no reason to go and check to see if the “real” value has change.

Note, you shouldn’t use Swing components (directly or indirectly) off of the AWT Event Dispatch Thread (EDT).

The simple fix is to make isRunning volatile and use EventQueue.invokeLater to invoke timeValue.setText.

Better would be to completely replace clockThread with a javax.swing.Timer (not java.util.Timer!), but that might be missing the point. At least make clockThread wait for isRunning to change.

Advertisement

Answer