21

Javadoc of the done() method of SwingWorker:

Executed on the Event Dispatch Thread after the doInBackground method is finished.

I've clues that it is not true in the case of canceled worker.
Done is called in each case (normal termination or cancellation) but when cancelled it is not enqueued to the EDT, as it happens for normal termination.

Is there some more precise analisis on when done is called in the case that a SwingWorker is cancelled?

Clarification: this question is NOT on how to cancel a SwingWorker. Here it is assumed that the SwingWorker is cancelled in the right way.
And it is NOT about thread still working when they are supposed to be finished.

S1LENT WARRIOR
  • 10,609
  • 4
  • 39
  • 55
AgostinoX
  • 6,923
  • 18
  • 72
  • 120
  • Why don't you have a look at the source? – Laurent Pireyn Jun 01 '11 at 16:04
  • That's odd. I have Java 6 and the docs say that SwingWorker is included in Java 6 but I can't find it in my rt.jar! :-/ – Aaron Digulla Jun 01 '11 at 16:08
  • Ah, it's in the src.zip but there is no .class file. Great.... – Aaron Digulla Jun 01 '11 at 16:09
  • @AgostinoX please check these two thread http://stackoverflow.com/questions/6171414/how-to-share-data-with-two2-swingworker-class-in-java/6186188#6186188 http://stackoverflow.com/questions/6051755/java-wait-cursor-display-problem/6060678#comment-7170467 and then please ask question, anyway tutorial about SwingWorker contains examples http://download.oracle.com/javase/tutorial/uiswing/concurrency/index.html – mKorbel Jun 01 '11 at 16:21
  • 3
    @mKorblel: I've read VERY carefully the tutorial on SwingWorker, and read the java doc too. If you have too, you'll know that the tutorial just shows a basic example of cancelling and not goes into details. – AgostinoX Jun 01 '11 at 16:48
  • @AgostinoX again in this thread is implemented method PropertyChangeListener http://stackoverflow.com/questions/6171414/how-to-share-data-with-two2-swingworker-class-in-java/6186188#6186188 – mKorbel Jun 01 '11 at 16:56
  • 2
    @mKorblel: sorry, mKorblel, have you read carefully my question? – AgostinoX Jun 01 '11 at 17:06
  • Related: http://stackoverflow.com/questions/24958793/swingworker-done-is-executed-before-process-calls-are-finished – kmort Jan 27 '15 at 21:53

6 Answers6

20

When a thread is cancelled via

myWorkerThread.cancel(true/false);

the done method is (quite surprisingly) called by the cancel method itself.

What you may expect to happen, but actually DOESN'T:
- you call cancel (either with mayInterrupt or not)
- cancel set up the thread cancellation
- the doInBackground exits
- the done is called*
(* the done is enqueued to the EDT, that means, if EDT is busy it happens AFTER the EDT has finished what it is doing)

What actually DOES happen:
- you call cancel (either with mayInterrupt or not)
- cancel set up the thread cancellation
- the done is called as part of cancel code*
- the doInBackground will exit when it will have finished its loop
(*the done isn't enqueued to the EDT, but called into the cancel call and so it has a very immediate effect on EDT, that often is the GUI)

I provide a simple example that proves this.
Copy, paste and run.
1. I generate a runtime exception inside done. The stack thread shows that done is called by cancel.
2. About after 4 seconds after cancelation, you'll recive a greeting from the doInBackground, that fhurterly proves that done is called before the thread exiting.

import java.awt.EventQueue;
import javax.swing.SwingWorker;

public class SwingWorker05 {
public static void main(String [] args) {
    EventQueue.invokeLater(new Runnable() {
        public void run() {
            try {
            W w = new W();
            w.execute();
            Thread.sleep(1000);
            try{w.cancel(false);}catch (RuntimeException rte) {
                rte.printStackTrace();
            }
            Thread.sleep(6000);
            } catch (InterruptedException ignored_in_testing) {}
        }

    });
}

public static class W extends SwingWorker <Void, Void> {

    @Override
    protected Void doInBackground() throws Exception {
        while (!isCancelled()) {
            Thread.sleep(5000);
        }
        System.out.println("I'm still alive");
        return null;
    }

    @Override
    protected void done() {throw new RuntimeException("I want to produce a stack trace!");}

}

}
AgostinoX
  • 6,923
  • 18
  • 72
  • 120
  • 2
    +1 The same (surprising) result as I got at about the same time ;-) – Howard Jun 01 '11 at 16:40
  • 3
    Thus it can even happen that you publish/process results after `done`. – Howard Jun 01 '11 at 16:42
  • maybe this one can help you http://stackoverflow.com/questions/6113944/how-cancel-the-execution-of-a-swingworker/6114890#6114890 – mKorbel Jun 01 '11 at 16:45
  • @Howard: where do we go from there? Two problem arises: 1) why is it undocumented 2) the most important thing. if in the end doInBackground releases sensitive resources, how am I sure that has finished? Do you suggest to open a new question? (please, who is reading and has not understood exactly this subtle feature, this is NOT the SwingWorker thread termination already discussed everywhere) – AgostinoX Jun 01 '11 at 17:08
  • @AgostinoX please see my question http://stackoverflow.com/questions/7053865/cant-get-arrayindexoutofboundsexception-from-future-and-swingworker-if-thread – mKorbel Aug 14 '11 at 00:13
  • 2
    This has been accepted as a bug by Oracle: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6826514 – Duncan Jones Feb 27 '12 at 12:31
  • @Duncan, thanks! this is very interesting, it is actually this issue. If I had known it at the time i put this question, there would not have been all this discussion. – AgostinoX Feb 29 '12 at 09:12
  • The documentation of SwingWorker suggest you to check the isCancelled() for verifying is the cancel flag was actived by a call to the cancel method. So i guess that its depends on you. If you remove the sleep, the programs finish.. that means that the doInBackground still running remains as an ignored thread for finishing. All the clean up job of the swing worker has to go in the done method. – Victor Aug 01 '13 at 04:38
  • 2
    That bug report hasn't been updated, but in Java 7 calling `#cancel()` does not call `#done()` (Oracle JDK) – searchengine27 Apr 26 '16 at 18:54
  • 2
    Sorry, @searchengine27, but that's not the case. Looking at the Java 8 JRE source right now; `SwingWorker#cancel()` calls `FutureTask#cancel()`, which **if it does not immediately return false** calls a private helper method `finishCompletion()`, which unconditionally calls `FutureTask#done()`. And SwingWorker's anonymous subclass of FutureTask overrides that `done()` to call its own private `SwingWorker#doneEDT()`, which unconditionally calls or enqueues its own `done()` on the EDT. – Ti Strga May 25 '18 at 19:26
6

done() is called in any case, wether the worker is cancelled or it finishes normally. Nevertheless there are cases where the doInBackground is still running and the done method is called already (this is done inside cancel() no matter if the thread already finished). A simple example can be found here:

public static void main(String[] args) throws AWTException {
    SwingWorker<Void, Void> sw = new SwingWorker<Void, Void>() {

        protected Void doInBackground() throws Exception {
            System.out.println("start");
            Thread.sleep(2000);
            System.out.println("end");
            return null;
        }

        protected void done() {
            System.out.println("done " + isCancelled());
        }
    };
    sw.execute();
    try {
        Thread.sleep(1000);
        sw.cancel(false);
        Thread.sleep(10000);
    } catch (InterruptedException ex) {
        ex.printStackTrace();
    }

Thus it can be the case that done is called before doInBackground finishes.

Howard
  • 36,183
  • 6
  • 60
  • 81
  • Ok. the code is almost the same that I provided. More, i proved that done happens inside cancel with the stack trace trick. – AgostinoX Jun 01 '11 at 17:02
1

Until the SwingWorker is fixed http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6826514 Here a simple (tested) version with the basic (similar) functions then SwingWorker

/*
 * To change this template, choose Tools | Templates
 * and open the template in the editor.
 */
package tools;

import java.util.LinkedList;
import java.util.List;
import javax.swing.SwingUtilities;

/**
 *
 * @author patrick
 */
public abstract class MySwingWorker<R,P> {

    protected abstract R doInBackground() throws Exception;
    protected abstract void done(R rvalue, Exception ex, boolean canceled);
    protected void process(List<P> chunks){}
    protected void progress(int progress){}

    private boolean cancelled=false;
    private boolean done=false;
    private boolean started=false;
    final private Object syncprogress=new Object();
    boolean progressstate=false;
    private int progress=0;
    final private Object syncprocess=new Object();
    boolean processstate=false;
    private LinkedList<P> chunkes= new LinkedList<>();

    private Thread t= new Thread(new Runnable() {
        @Override
        public void run() {
            Exception exception=null;
            R rvalue=null;
            try {
                rvalue=doInBackground();
            } catch (Exception ex) {
                exception=ex;
            }

            //Done:
            synchronized(MySwingWorker.this)
            {
                done=true;
                final Exception cexception=exception;
                final R crvalue=rvalue;
                final boolean ccancelled=cancelled;

                SwingUtilities.invokeLater(new Runnable() {
                    @Override
                    public void run() {
                        done(crvalue, cexception, ccancelled);
                    }
                });
            }

        }
    });    

    protected final void publish(P p)
    {
        if(!Thread.currentThread().equals(t))
            throw new UnsupportedOperationException("Must be called from worker Thread!");
        synchronized(syncprocess)
        {
            chunkes.add(p);
            if(!processstate)
            {
                processstate=true;
                SwingUtilities.invokeLater(new Runnable() {
                    @Override
                    public void run() {
                        List<P> list;
                        synchronized(syncprocess)
                        {
                            MySwingWorker.this.processstate=false;
                            list=MySwingWorker.this.chunkes;
                            MySwingWorker.this.chunkes= new LinkedList<>();
                        }
                        process(list);
                    }
                });
            }
        }
    }

    protected final void setProgress(int progress)
    {
        if(!Thread.currentThread().equals(t))
            throw new UnsupportedOperationException("Must be called from worker Thread!");
        synchronized(syncprogress)
        {
            this.progress=progress;
            if(!progressstate)
            {
                progressstate=true;
                SwingUtilities.invokeLater(new Runnable() {
                    @Override
                    public void run() {
                        int value;
                        //Acess Value
                        synchronized(syncprogress)
                        {
                            MySwingWorker.this.progressstate=false;
                            value=MySwingWorker.this.progress;
                        }
                        progress(value);
                    }
                });
            }
        }
    }

    public final synchronized void execute()
    {
        if(!started)
        {
            started=true;
            t.start();
        }
    }

    public final synchronized boolean isRunning()
    {
        return started && !done;
    }

    public final synchronized boolean isDone()
    {
        return done;
    }

    public final synchronized boolean isCancelled()
    {
        return cancelled;
    }

    public final synchronized void cancel()
    {
        if(started && !cancelled && !done)
        {
            cancelled=true;
            if(!Thread.currentThread().equals(t))
                t.interrupt();
        }
    }

}
Jeff
  • 131
  • 8
pknoe3lh
  • 362
  • 3
  • 10
1

something is possible, other could be illusion

really nice outPut

run:
***removed***
java.lang.RuntimeException: I want to produce a stack trace!
        at help.SwingWorker05$W.done(SwingWorker05.java:71)
        at javax.swing.SwingWorker$5.run(SwingWorker.java:717)
        at javax.swing.SwingWorker.doneEDT(SwingWorker.java:721)
        at javax.swing.SwingWorker.access$100(SwingWorker.java:207)
        at javax.swing.SwingWorker$2.done(SwingWorker.java:284)
        at java.util.concurrent.FutureTask$Sync.innerCancel(FutureTask.java:293)
        at java.util.concurrent.FutureTask.cancel(FutureTask.java:76)
        at javax.swing.SwingWorker.cancel(SwingWorker.java:526)
        at help.SwingWorker05$1.run(SwingWorker05.java:25)
        at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:209)
        at java.awt.EventQueue.dispatchEvent(EventQueue.java:597)
        at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:269)
        at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:184)
        at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:174)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:169)
        at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:161)
        at java.awt.EventDispatchThread.run(EventDispatchThread.java:122)
I'm still alive
Thread Status with Name :SwingWorker1, SwingWorker Status is STARTED
SwingWorker by tutorial's background process has completed
Thread Status with Name :SwingWorker1, SwingWorker Status is DONE
Thread Status with Name :look here what's possible with SwingWorker, SwingWorker Status is STARTED
BUILD SUCCESSFUL (total time: 10 seconds)

from

import java.awt.EventQueue;
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import javax.swing.SwingWorker;

public class SwingWorker05 {

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

            public void run() {
                try {
                    W w = new W();
                    w.addPropertyChangeListener(
                            new SwingWorkerCompletionWaiter("look here what's possible with SwingWorker"));
                    w.execute();
                    Thread.sleep(1000);
                    try {
                        w.cancel(false);
                    } catch (RuntimeException rte) {
                        rte.printStackTrace();
                    }
                    Thread.sleep(6000);
                } catch (InterruptedException ignored_in_testing) {
                }
            }
        });

        final MySwingWorker mySW = new MySwingWorker();
        mySW.addPropertyChangeListener(new SwingWorkerCompletionWaiter("SwingWorker1"));
        mySW.execute();
    }

    private static class MySwingWorker extends SwingWorker<Void, Void> {

        private static final long SLEEP_TIME = 250;

        @Override
        protected Void doInBackground() throws Exception {
            Thread.sleep(SLEEP_TIME);
            return null;
        }

        @Override
        protected void done() {
            System.out.println("SwingWorker by tutorial's background process has completed");
        }
    }

    public static class W extends SwingWorker {

        @Override
        protected Object doInBackground() throws Exception {
            while (!isCancelled()) {
                Thread.sleep(5000);
            }

            System.out.println("I'm still alive");
            return null;
        }

        @Override
        protected void done() {
            System.out.println("***remove***");
            throw new RuntimeException("I want to produce a stack trace!");
        }
    }

    private static class SwingWorkerCompletionWaiter implements PropertyChangeListener {

        private String str;

        SwingWorkerCompletionWaiter(String str) {
            this.str = str;
        }

        @Override
        public void propertyChange(PropertyChangeEvent event) {
            if ("state".equals(event.getPropertyName()) && SwingWorker.StateValue.DONE == event.getNewValue()) {
                System.out.println("Thread Status with Name :" + str + ", SwingWorker Status is " + event.getNewValue());
            } else if ("state".equals(event.getPropertyName()) && SwingWorker.StateValue.PENDING == event.getNewValue()) {
                System.out.println("Thread Status with Mame :" + str + ", SwingWorker Status is " + event.getNewValue());
            } else if ("state".equals(event.getPropertyName()) && SwingWorker.StateValue.STARTED == event.getNewValue()) {
                System.out.println("Thread Status with Name :" + str + ", SwingWorker Status is " + event.getNewValue());
            } else {
                System.out.println("Thread Status with Name :" + str + ", Something wrong happends ");
            }
        }
    }
}
mKorbel
  • 108,320
  • 17
  • 126
  • 296
  • @mKorbel.I'm working on your sample.First question.Since my implementation of SwingWorker methods is 6 rows, can you point out where the "wrong implementation" is?Second.I have never used "bug" word.If you read the question, you'll see that is about when the cancel is called.I've shown beyond any doubt that yes, done is called in each case(in EDT as expected!) AND in the case of cancellation it is not enqueued to EDT but is called as part of cancel(...) call,potentially before exiting from doInBackground method.Now I'm verifying the exact behavior of PropertyChangeListener, I'll let you know. – AgostinoX Jun 02 '11 at 15:04
  • @AgostinoX :-) Since my implementation of SwingWorker methods is 6 rows :-) no, not that's just plain vanilla innerClass or voids 1st. waiting for timer, 2nd. for 1st ends, nothing esle, small mistake..., let’s go back to the start (Coldplay), most important if you want to play with SwingWorker then your code could be confirm its output directly to the GUI (JTextArea), otherwise doesn't make me sense, because System.out.print() work in all cases (+ - CitiBus), much luck – mKorbel Jun 02 '11 at 20:31
  • Ok. so the W class is right. Do you mind remove the comment about "wrong implementation of classes"? Thanks. Then you say problem is waiting for timer? Which timer? Perhaps you mean the Thread.sleep. I've made a class that everyone can use to reproduce the specific problem i'm talking about, so i need something that "hang" for some seconds like a slow database connection. I'm sure that it can be done in better ways, so I'm glad to hear from you. – AgostinoX Jun 03 '11 at 08:23
  • @@AgostinoX sure I'll/can remove that just and only by your ask but nothing gonna change about that correct syntax must include extends SwingWorker//..., otherwise that not SwingWorker as we knew, only some class that's required some methods but without declared SWingWorkers funcionalities ..., so finally now I'm outta from this thread – mKorbel Jun 03 '11 at 08:42
  • @mKorbel.I, by your ask too, add to my SwingWorker implementation, as it would help people used to see generics to understand well the class. But I want to pointout that he functionalities are not provided by generics but by extending a class or implementing an interface. Generics are a way to avoid typecasts. Moreover, since they're implemented by erasure, the compiled code with or without is the same; we are speaking about convenience and habits, not correctness. – AgostinoX Jun 03 '11 at 10:23
0

From the Java docs: cancel(boolean mayInterruptIfRunning) "mayInterruptIfRunning - true if the thread executing this task should be interrupted; otherwise, in-progress tasks are allowed to complete"

If you call cancel(true) instead of cancel(false) that seems to behave as you are expecting.

I have not seen done() called off the EDT using EventQueue.isDispatchThread()

0

IF you use return Void: ...@Override public Void doInBackground(){...

done() is invoked when doInBackground() has finished.

IF you don't use return Void: ...@Override public boolean doInBackground(){...

done() is ignored and you know have finished cause you have your return.

IvanVar
  • 1
  • 1
  • 1
    Welcome to SO. This post does not meet our quality standards, how to write good quality answers read [here](https://stackoverflow.com/help/how-to-answer). – Rahul Sharma Apr 27 '17 at 02:03