Updating GUI through threads

Question

I'm fairly new to threads and haven't written Java in a long time so bear with me here. I have a very simple GUI. It has a counter, a status label, and two buttons, start and stop respectively.

What I wanted it to do was update my status label using a counter thread. When I hit start it supposedly starts the counter at 0 and increments it every second, and when I choose to hit stop it should suspend the current thread and wait for the start button to be pressed again. However whenever I hit stop it just suspends an waits for a second and resumes the counting. When in reality I want it to stay suspended. I'm not really sure why it's doing that, tried searching it before posting here but got nothing. Also feel free to criticize on anything you'd like.

Here's what I have:

UPDATED AS PER @MadProgrammer's answer.

import java.awt.Font;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JFrame;
import static javax.swing.JFrame.EXIT_ON_CLOSE;
import javax.swing.JLabel;
import javax.swing.SwingUtilities;

public class main extends JFrame 
{


    JLabel countLabel = new JLabel("0");
    JLabel statusLabel = new JLabel("Task not completed.");
    JButton startButton = new JButton("Start");
    JButton stopButton = new JButton("Stop");
    CounterThread worker = new CounterThread("worker", countLabel, statusLabel);

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {

            @Override
            public void run() {
                new Main("Counter Demo");
            }
        });
    }

    public Main(String title) {
        super(title);

        setLayout(new GridBagLayout());

        countLabel.setFont(new Font("serif", Font.BOLD, 28));

        GridBagConstraints gc = new GridBagConstraints();

        gc.fill = GridBagConstraints.NONE;

        gc.gridx = 0;
        gc.gridy = 0;
        gc.weightx = 1;
        gc.weighty = 1;
        add(countLabel, gc);

        gc.gridx = 0;
        gc.gridy = 1;
        gc.weightx = 1;
        gc.weighty = 1;
        add(statusLabel, gc);

        gc.gridx = 0;
        gc.gridy = 2;
        gc.weightx = 1;
        gc.weighty = 1;
        add(startButton, gc);

        gc.gridx = 0;
        gc.gridy = 3;
        gc.weightx = 1;
        gc.weighty = 1;
        add(stopButton, gc);

        startButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {
                worker.start();
                //notify();
            }
        });
        stopButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {
                worker.suspend();
            }
        });
        setSize(200, 400);
        setDefaultCloseOperation(EXIT_ON_CLOSE);
        setVisible(true);
    }

public class CounterThread implements Runnable {

    public Thread t;
    public String threadName;
    boolean suspended = false;
    JLabel countLabelName;
    JLabel statusLabelName;

    CounterThread(String name, JLabel cLabel, JLabel sLabel) {
        this.threadName = name;
        this.countLabelName = cLabel;
        this.statusLabelName = sLabel;
    }

    public void run() {
        try {
            // Simulate doing something useful.
            for (int i = 0; i <= 10; i++) {
                synchronized (this) {
                    if (suspended) 
                    {             
                        wait();
                    }
                }
                final int count = i;

                SwingUtilities.invokeLater(new Runnable() {
                    public void run() {
                        countLabelName.setText(Integer.toString(count));
                    }
                });
                Thread.sleep(1000);
            }
        } catch (InterruptedException e) {
        }

        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                statusLabelName.setText("Completed.");
            }
        });

        this.start();
    }

    public boolean getStatus() {
        return t == null;
    }

    public void start() {
        if (getStatus()) {
            //t = new Thread(new CounterThread(this.threadName, this.countLabelName, this.statusLabelName));
            t = new Thread(this);
            t.start();
        }
    }

    public void suspend() {
        statusLabelName.setText("Task is paused");
        suspended = true;
    }
    //create an object whose only purpose is to synchronize

    synchronized void resume() {
        statusLabelName.setText("Task has resumed");
        suspended = false;
        this.notify();
    }

}
}

Your best bet is to review the Swing and threading tutorial at oracle and check out the relevant swing utility classes, some of which you're importing but never using. Similarly, you seem to be aware of java.util.concurrent but are not using it. Avoid direct use of synchronization primitives, `notify`, etc. Clean up your code and its indentation and - you have a class that implements runnable but contains a thread, and starts a thread, it's just hard to follow what's going on, probably for you as well. — pvg, Sep 14 '17 at 03:30
Unless you have a particular need to do some additional processing in the background, I'd consider having a look at using a Swing `Timer` instead — MadProgrammer, Sep 14 '17 at 03:47
`boolean suspended = false;` should probably also be marked `volatile` (or use a `AtomicBoolean`) — MadProgrammer, Sep 14 '17 at 03:49
`if (getStatus()) {` is causing the `wait` to be skipped because it's returning `false` (`t == null` which is `false` while it's running) — MadProgrammer, Sep 14 '17 at 03:53
@MadProgrammer why would it need to be volatile ? I've personally never had to use this before. Also would a Swing `timer` allow for a delay until a button is pressed ? — A curious one, Sep 14 '17 at 03:55
@MadProgrammer `if (getStatus()) {` returns true if the thread is suspended, which has to be true otherwise it wouldn't go into that `if-statement` — A curious one, Sep 14 '17 at 03:58
`volatile` will make sure that each `Thread` is reading the actual value (and not a cached copy) and yes, you can easily suspend a `Timer`, have a look at the [JavaDocs](https://docs.oracle.com/javase/8/docs/api/javax/swing/Timer.html) and [How to use Swing Timers](https://docs.oracle.com/javase/tutorial/uiswing/misc/timer.html) for more details — MadProgrammer, Sep 14 '17 at 03:59
@Acuriousone Actually, it returns `true` if `t` is `null`, which raises the question of why would you still be in the `run` method - I don't think it's a valid check to make, using a `enum` would be a better choice and it would carry more meaning (and provide for more possibilities) - And just so were clear - you never set `t` to `null` anyway — MadProgrammer, Sep 14 '17 at 03:59

score 0 · Answer 1 · edited Jun 20 '20 at 09:12

Basically...

synchronized(this)
{
    if(suspended)
    {
        if(getStatus())
            wait();
        resume();
    }
}

getStatus is returning false so it's skipping the wait call (because t != null.

I'm not really sure why need to check this, but I might have a enum or other flag which returns a more meaningful state (like RUNNING, STOPPED, PAUSED ... what ever)

I was able to make it work by doing something like...

synchronized(this)
{
    if(suspended)
    {
        wait();
    }
}

instead.

Having said that though. I'd personally consider using a Swing Timer which would do all this work for you and would trigger it's updates within the context of the EDT

Updated after original code was modified

Even with your suggested answer it's still behaving the same way, it suspends it for a brief second and resumes right away

You modified the code from the original post, adding

t = new Thread(new CounterThread(this.threadName, this.countLabelName, this.statusLabelName));

to the start method, but your UI code already has a reference to CounterThread which it's interacting with, so now you have two instances of the same class, one which is running in the background ticking away and one which your UI code is interacting with.

So when the UI calls suspend, it's not changing the suspended state of the instance which is actually running

import java.awt.Font;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JFrame;
import static javax.swing.JFrame.EXIT_ON_CLOSE;
import javax.swing.JLabel;
import javax.swing.SwingUtilities;

public class Main extends JFrame {

    JLabel countLabel = new JLabel("0");
    JLabel statusLabel = new JLabel("Task not completed.");
    JButton startButton = new JButton("Start");
    JButton stopButton = new JButton("Stop");
    int holder;
    CounterThread worker = new CounterThread("worker", countLabel, statusLabel);

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {

            @Override
            public void run() {
                new Main("Counter Demo");
            }
        });
    }

    public Main(String title) {
        super(title);

        setLayout(new GridBagLayout());

        countLabel.setFont(new Font("serif", Font.BOLD, 28));

        GridBagConstraints gc = new GridBagConstraints();

        gc.fill = GridBagConstraints.NONE;

        gc.gridx = 0;
        gc.gridy = 0;
        gc.weightx = 1;
        gc.weighty = 1;
        add(countLabel, gc);

        gc.gridx = 0;
        gc.gridy = 1;
        gc.weightx = 1;
        gc.weighty = 1;
        add(statusLabel, gc);

        gc.gridx = 0;
        gc.gridy = 2;
        gc.weightx = 1;
        gc.weighty = 1;
        add(startButton, gc);

        gc.gridx = 0;
        gc.gridy = 3;
        gc.weightx = 1;
        gc.weighty = 1;
        add(stopButton, gc);

        startButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {
                worker.start();
            }

        });

        stopButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {
                worker.suspend();
            }
        });
        setSize(200, 400);
        setDefaultCloseOperation(EXIT_ON_CLOSE);
        setVisible(true);
    }

    public class CounterThread implements Runnable {

        public Thread t;
        public String threadName;
        boolean suspended = false;
        JLabel countLabelName;
        JLabel statusLabelName;

        CounterThread(String name, JLabel cLabel, JLabel sLabel) {
            this.threadName = name;
            this.countLabelName = cLabel;
            this.statusLabelName = sLabel;
        }

        public void run() {
            try {
                // Simulate doing something useful.
                for (int i = 0; i <= 10; i++) {
                    synchronized (this) {
                        if (suspended) {
                            wait();
                        }
                    }
                    final int count = i;

                    SwingUtilities.invokeLater(new Runnable() {
                        public void run() {
                            countLabelName.setText(Integer.toString(count));
                        }
                    });

                    Thread.sleep(1000);

                }
            } catch (InterruptedException e) {
            }

            SwingUtilities.invokeLater(new Runnable() {
                public void run() {
                    statusLabelName.setText("Completed.");
                }
            });

            this.start();
        }

        public boolean getStatus() {
            return t == null;
        }

        public void start() {
            if (getStatus()) {
                //t = new Thread(new CounterThread(this.threadName, this.countLabelName, this.statusLabelName));
                t = new Thread(this);
                t.start();
            }
        }

        public void suspend() {
            statusLabelName.setText("Task is paused");
            suspended = true;
        }
        //create an object whose only purpose is to synchronize

        synchronized void resume() {
            statusLabelName.setText("Task has resumed");
            suspended = false;
            this.notify();
        }

    }
}

Also, I don't see how using a Swing Timer would help me in this case, since there's no actual delay for the wait

Then you clearly don't understand how a Timer works

import java.awt.Font;
import java.awt.GridBagConstraints;
import java.awt.GridBagLayout;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JButton;
import javax.swing.JFrame;
import static javax.swing.JFrame.EXIT_ON_CLOSE;
import javax.swing.JLabel;
import javax.swing.SwingUtilities;
import javax.swing.Timer;

public class Main extends JFrame {

    JLabel countLabel = new JLabel("0");
    JLabel statusLabel = new JLabel("Task not completed.");
    JButton startButton = new JButton("Start");
    JButton stopButton = new JButton("Stop");
    int holder;

    Timer timer;
    int count = 0;

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {

            @Override
            public void run() {
                new Main("Counter Demo");
            }
        });
    }

    public Main(String title) {
        super(title);

        setLayout(new GridBagLayout());

        countLabel.setFont(new Font("serif", Font.BOLD, 28));

        GridBagConstraints gc = new GridBagConstraints();

        gc.fill = GridBagConstraints.NONE;

        gc.gridx = 0;
        gc.gridy = 0;
        gc.weightx = 1;
        gc.weighty = 1;
        add(countLabel, gc);

        gc.gridx = 0;
        gc.gridy = 1;
        gc.weightx = 1;
        gc.weighty = 1;
        add(statusLabel, gc);

        gc.gridx = 0;
        gc.gridy = 2;
        gc.weightx = 1;
        gc.weighty = 1;
        add(startButton, gc);

        gc.gridx = 0;
        gc.gridy = 3;
        gc.weightx = 1;
        gc.weighty = 1;
        add(stopButton, gc);

        timer = new Timer(1000, new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
                count++;
                countLabel.setText(Integer.toString(count));
            }
        });

        startButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {
                timer.start();
            }

        });

        stopButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {
                if (timer.isRunning()) {
                    timer.stop();
                    stopButton.setText("Resume");
                } else {
                    timer.restart();
                    stopButton.setText("Stop");
                }
            }
        });
        setSize(200, 400);
        setDefaultCloseOperation(EXIT_ON_CLOSE);
        setVisible(true);

    }
}

Now, there is the pause and resume issues all taken care off

I see, I was under the impression `return t==null` would return me the state of the thread, aka RUNNING, STOPPED. Even with your suggested answer it's still behaving the same way, it suspends it for a brief second and resumes right away — A curious one, Sep 14 '17 at 04:03
You've changed the code from the original post. In the `start` method, you're doing `t = new Thread(new CounterThread(this.threadName, this.countLabelName, this.statusLabelName));`, but your UI code is interacting with a different instance - `CounterThread worker = new CounterThread("worker", countLabel, statusLabel);` - there's no way you can now modify the code which is actually running the thread (I changed the names) — MadProgrammer, Sep 14 '17 at 04:07
At least it stays suspended now, I just need to figure out how to properly use `notify` to resume the counter once the start button is pressed again. Also, I don't see how using a Swing Timer would help me in this case, since there's no actual delay for the wait. I'm simply waiting until the start button is clicked again — A curious one, Sep 14 '17 at 04:41
Ah, yes very nice answer. Although that kind of neglects the whole point of me trying to work with threads. I would still very much like to have my own thread class and use it to update the GUI. Maybe I should've been more clear on that. — A curious one, Sep 14 '17 at 19:08
@Acuriousone I’m just throwing the example out there - I’d also consider having a look SwingWorker — MadProgrammer, Sep 14 '17 at 19:38

Updating GUI through threads

1 Answers1

Updated after original code was modified