Review of solution for Cancelled Tasks

Roman Kennke roman at
Thu Jan 5 12:29:25 PST 2012

Hi there,

> The InterruptedException is only thrown if somebody has a blocking call within the "call" method using streams or Thread.sleep or whatnot. Otherwise you get no notification in the call method itself but have to manually ask "is this cancelled?".

Interestingly, I/O streams are one of the things that are not
interruptible (i.e. don't throw InterruptedException, and when you call
Thread.interrupt() on a thread that's in an IO, it won't do anything).
You can get interuptible IO only by using NIO. You can still check
Thread.isInterrupted() though (but only after the IO operation

> If the update methods throw an exception (maybe even InterruptedException...?) then it provides a way for most tasks to know that the thing has been cancelled. But tasks that never update progress or the message or whatnot would still run free if they aren't checking cancelled. But there's nothing we can do there.
> I tend to agree with Jim that a checked exception would have been appropriate here. It forces people (annoys them too) to fess up to the fact that this thing might have been cancelled. However that will break people even more at this point (source level breakage).
> Another benefit to having the updateXXX methods throw an exception when called from a background thread of a cancelled task is when you write non-looping background code:
> doSomething();
> updateProgress(1, 4);
> doSomething2();
> updateProgress(2, 4);
> doSomething3();
> updateProgress(3, 4);
> doSomething4();
> updateProgress(4, 4);
> In such a case, having to check isCancelled after each doSomething before calling updateProgress is kind of a pain. Instead having an exception lets my code remain simple and easy to read. Ideally we would have a checked TaskCancelledException so that:
> try {
>     doSomething();
>     updateProgress(1, 4);
>     doSomething2();
>     updateProgress(2, 4);
>     doSomething3();
>     updateProgress(3, 4);
>     doSomething4();
>     updateProgress(4, 4);
> } catch (TaskCancelledException ex) {
>     // do stuff to clean up
>     // ...
>     // Update the progress and message
>     Platform.runLater(() -> {
>         updateProgress(4, 4);
>         updateMessage("Cancelled");
>     });
> }
> However, I think it is too late to add a checked exception. But we could add the unchecked exception. It will still break some people, but probably not many? I worry about breaking anybody. However if we threw an unchecked exception I could still write the above code if I care about responding to the cancellation in the task, and if I don't, I just let the exception kill the thread and I'm happy as a clam. And if instead of a TaskCancelledException I just threw InterruptedException, then I have a single exception I catch as an indication of cancellation instead of potentially two exceptions.
> SO I think the decision has to be:
> 	1) Throw an unchecked exception from updateXXX whenever called from background thread that has been cancelled (preference might be for InterruptedException)
> 	or 
> 	2) Simply document that you have to check for isCancelled and leave it to the developer

Another option would be to call Thread.interrupt() to signal that the
thread has been interrupted. Any call to a method that is blocking would
throw an InterruptedException then. App code can check
Thread.isInterrupted() if it wants to bail out even if it doesn't call
blocking code. I am not even sure that we need a notion of 'isCancelled'
if the interrupted flag does exactly this. On the other hand, we might
want to use the interrupted flag internally, and - for example - throw
an unchecked TaskInterruptedException from JavaFX code, as well as
blowing up in blocking calls with InterruptedException.

What do you think?

Regards, Roman

More information about the openjfx-dev mailing list