SwingWorker, Cancel-Button doesn't work

Question

I have a window, with a Start- and Stop-Button. The Start-Button starts the algorithm and the stop-button should stop it. I use SwingWorker do run the algorithm in the background and normally calling worker.cancel(true) should stop the algorithm running. I also have a Label, that visualize the Status, e.g. if I press "Stop", then the Labeltext changes to "stopped", so the Problem isnt on the actionLister of the Button.

My code looks like this:

public class MainWindow extends JFrame implements ActionListener, WindowListener
{
 // Some code, like generating JFrame, JButtons and other stuff not affencting the task.

 Worker worker = new Worker();

    public void actionPerformed(ActionEvent e)
    {       
    boolean isStarted = false;



    // Start Button
    if (e.getSource() == this.buttonStart)
    {
        if(!isStarted)
        {
        System.out.println("start");
        labelSuccess.setText("Mapping started!");
        this.setEnabled(true);
        worker.execute();
        isStarted = false;
        }
    }
    // Stop Button
    if (e.getSource() == this.buttonStop)
        {
        labelSuccess.setText("Mapping stopped!");
        worker.cancel(true);
        }
  }

class Worker extends SwingWorker<Void, Void> {

    @Override
    protected Void doInBackground() throws Exception {

    if(!isCancelled())
    {
        mapp();
        Thread.sleep(60);
    if (isCancelled()) {
     System.out.println("SwingWorker - isCancelled");
    }
    }
        return null;
    }

}

At this Point, pressing the Stop-Button causes just a change of the Label-Text, but the algorithm in the background is still running. This now bothers me for quite a while and I just can't get it going.

Thanks a lot for any help, much appreciated.

edit1: I generate a new instance of worker now outside of actionPerformed, so now there is no new Worker generated on every mouse click.

Answer 1

Maybe if you use while instead of if on doInBackground() method of Worker class you will solve your problem. You must to put out of the while loop the mapp(), because you only want to invoke it one time. You should do something like this:

class Worker extends SwingWorker<Void, Void> {

    @Override
    protected Void doInBackground() throws Exception {

    mapp();    
    while(!isCancelled()){
        Thread.sleep(60);
    }
     System.out.println("SwingWorker - isCancelled");    
        return null;       
}

This link could be useful to understanding how to use SwingWorker.

EDIT:

As you can see on another questions like this or this, using SwingWorker has some problems to manage the cancel method, because this method Attempts to cancel execution of this task. This attempt will fail if the task has already completed, has already been cancelled, or could not be cancelled for some other reason, like Oracle explains, and those "some other reasons" are discussed on the links I've posted.

You can do solve your problem using directly Threads. Your code would be something like this:

    public class MainWindow extends JFrame implements ActionListener, WindowListener
    {
     // Some code, like generating JFrame, JButtons and other stuff not affencting the task.

        final Thread th1 = new Thread(new Runnable() {

                    @Override
                    public void run() {
                        mapp();                 

                    }
                }); 
        public void actionPerformed(ActionEvent e)
        {       
        boolean isStarted = false;

    // Start Button
    if (e.getSource() == this.buttonStart)
    {
        if(!isStarted)
        {
        System.out.println("start");
        labelSuccess.setText("Mapping started!");
        this.setEnabled(true);
        th1.start();
        isStarted = false;
        }
    }
    // Stop Button
    if (e.getSource() == this.buttonStop)
        {
        labelSuccess.setText("Mapping stopped!");
        th1.stop();
        }
  }

This solutions uses the method stop(), which is deprecated, but it works. I've tried using interrupt(), but I don't know why the thread ran till finish the execution of mapp(). Obviously, using stop() is not the best method but it works stopping the mapp() execution before it finishes.

I recommend you to learn more about SwingWorker, Thread and Task to find the best solution to your problem.

Answer 2

Your problem is there is no loop in the worker: if you want to cancel a process using a flag, that process should check the flag from time to time, so if your method Worker.mapp() has to be stopped, check the flag there, no just before and after calling it.