SwingWorker not stopping when using isCancelled()

Question

I've created a gui that has 3 buttons; connect, disconnect, abort. They all share a common SwingWorker thread, but for some reason the thread never finishes unless I hit the abort button.

I didn't write any code for the disconnect button yet, but even so, it's not relevant to my question. However, why doesn't the thread just finish and print the Done statement? My guess is that it's getting stuck in the this.isCancelled() loop, but even if I do:

while(!this.isCancelled() || !this.isDone()) { // run loop }

it never prints Done

The thread is only finished once I hit the abort button. I'm using the isCancelled() loop method because this seems to be the recommended way of canceling a SwingWorker.

I'm posting only the relevant code below because there are literally hundreds of lines.

   private class ButtonListener implements ActionListener {

      private MyWorker worker;

      @Override
      public void actionPerformed(ActionEvent e) {

         if(e.getSource().equals(connectButton)) { 

            worker = new MyWorker(activityTextArea);
            worker.execute();
            activityTextArea.append("Connect Button Pressed"); 
         }

         if(e.getSource().equals(disconnectButton)) { 

            activityTextArea.append("Disconnect Button Pressed"); 
         }

         if(e.getSource().equals(abortButton)) { 

            worker.cancel(true);
            activityTextArea.append("Abort Button Pressed"); 
         }
      }
   }

Here is the SwingWorker:

public class MyWorker extends SwingWorker<Void, Void> {

   private JTextArea textArea;

   MyWorker(JTextArea textArea) {
      this.textArea = textArea;
   }

   private void startWorker() {

      ProcessBuilder pb = new ProcessBuilder();
      pb.command("aCmd");

      Process p;

      try {
         p = pb.start();

         BufferedReader br = new BufferedReader(new InputStreamReader(p.getInputStream()));

         String line;

         while((line = br.readLine()) != null) {
            textArea.append(line + "\n");
         }
         br.close();
      } catch (IOException e) {
         // TODO Auto-generated catch block
         e.printStackTrace();
      }
   }

   @Override
   protected Void doInBackground() throws Exception {
      // TODO Auto-generated method stub
      boolean isFirstTime = true;

      while(!this.isCancelled()) {

         if(this.isDone()) { break; }

         Thread.sleep(3000);

         if(isFirstTime) {
            startWorker();
            isFirstTime = false;
         } 
      }

      return null;
   }

   @Override
   protected void done() {
      textArea.append("Done\n");
   }
}

Raz · Accepted Answer · 2014-08-17T03:07:14.260

The issue here is in the implementation of MyWorker.doInBackground() method, isDone() is being used to control the execution of the loop. This is probably not how you intend the doInBackground() to work.

To really understand whats going on, we need to consider the behaviour of isDone() and the lifecycle of the SwingWorker.

The isDone() method returns true for these scenarios:

After done() has finished executing
The worker has been cancelled
Abnormally termination due to some Exception.

SwingWorker LifeCycle

PENDING: The state during the period from the construction of the object until just before doInBackground is invoked.
STARTED: The state during the period from shortly before doInBackground() is invoked until shortly before done() is invoked. The SwingWorker is allocated a Worker Thread before entering this state, and this Worker Thread executes the doInBackground().
- While executing doInBackground() in this state, the worker can send intermediate results by using publish(..), which then gets process(..) by the Event Dispatch Thread (EDT).
DONE: Transition to this state happens after the done() has finished executing in the Event Dispatch Thread (EDT). This is the final state for the remainder of the existence of the object.

It is important to understand the above workflow in order to write a performant and glitch free GUI.

Based on the above discussion, you probably want to replace if(this.isDone()) { break; } with something that reflects the completion of your intended processing.

Another issue here is that you're doing swing related activities in a method that is invoked by doInBackground(). The textArea.append(line + "\n"); should happen in a process(..) method which is executed in the Event Dispatch Thread, as shown below:

public class MyWorker extends SwingWorker<Void, String> {

    @Override
    protected void process(List<String> chunks) {
        for (String line : chunks) {
            textArea.append(line + "\n"); // display intermediate results from publish()
        }
    }

    private void startWorker() throws IOException {

        ProcessBuilder pb = new ProcessBuilder("aCmd");
        BufferedReader br = null;

        try {
            Process p = pb.start();
            br = new BufferedReader(new InputStreamReader(p.getInputStream()));
            String line;
            while ((line = br.readLine()) != null) {
                publish(line); // publish a chunk of result to process(..)
            }
        } catch (IOException e) {
            e.printStackTrace();
        } finally {
            if (br != null) br.close();
        }
    }

Reference: http://docs.oracle.com/javase/7/docs/api/javax/swing/SwingWorker.html

SwingWorker not stopping when using isCancelled()

1 Answers1