4

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.

carexcer
  • 1,407
  • 2
  • 15
  • 27
Alika87
  • 301
  • 1
  • 5
  • 22
  • If there is a loop in mapp() you have to handle an InterruptedException there. I don't think you'll come to your "SwingWorker isCancelled" otherwise. – Leo Pflug Jan 20 '14 at 14:23
  • 1
    you are creating an instance of worker in each `button click` see `Worker worker = new Worker();` they are not related the one you cancel that the one you want to cancel. – nachokk Jan 20 '14 at 14:33

2 Answers2

3

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.

Community
  • 1
  • 1
carexcer
  • 1,407
  • 2
  • 15
  • 27
  • thanks for the answer, but I tried that also before and it didnt work. – Alika87 Jan 20 '14 at 16:47
  • I guess the problem is there. Mapp() calls a different Method and most of the work is done there. But I dont get it, the worker processes this methods in the background, so canceling the worker, should also cancel the processing of that method. – Alika87 Jan 20 '14 at 17:38
  • I've edited the answer. I don't know why your mapp() doesn't stops, but maybe this new answer guides you on the right way. – carexcer Jan 20 '14 at 18:29
  • In your first snippet: You could as well not support cancelling, because `mapp()` will always execute till it is done. Cancelling only makes sense if you can brake the whole execution into parts. After (or before) execution of a part you check the cancel-flag and do so if signaled. Just executing everything and waiting _until_ the task is cancelled will end up in an infinite loop if the task is _not_ cancelled. – Fildor Jan 21 '14 at 08:08
  • Second snippet: please don't call `stop`. You can interrupt it, you can use CountDownLatch ... – Fildor Jan 21 '14 at 08:17
  • Fildor is right, after putting `mapp()` into a loop, the method runs infinetely. It processes normally, but after finishing, it is captured in an infinite loop, where the solution is displayed in every iteration. From there on it is possible to cancel the Swingworker, but not before `mapp()` finishes and gives the first solution – Alika87 Jan 21 '14 at 11:21
  • Of course, you mustn't put the `mapp()` inside the loop. You must put it before the loop, but with your solution for `SwingWorker` you can't stop the running of `mapp()` till it will be completed. Therefore I've given you another solution that it stops the `mapp()` execution before than it finishes. It's not the best way, but it works because I've tested it. Try looking for another way like using CountDownLatch as Fildor said. – carexcer Jan 21 '14 at 11:27
  • I fixed the problem, but different than i intented. I put the mapp() in a while-loop with a boolean-flag and by pressing stop, the state of the flag changes and the loop stop. Its not like canceling the worker, but it works really well. So I stick to that solution for now. – Alika87 Jan 23 '14 at 10:07
1

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.

Pablo Lozano
  • 10,122
  • 2
  • 38
  • 59
  • 3
    AND: he is calling execute and cancel on two different instances of worker. Mind the context, "worker" is created in. Each click on a button will create a new Worker. – Fildor Jan 20 '14 at 14:29