Review of solution for Cancelled Tasks
Janda Martin
jandam at crcdata.cz
Wed Jan 4 00:05:29 PST 2012
I agree with Tom Eugelink. After cancelling task you should be able to fully control progress and status text. For example db application you rollback transaction.
I think that IllegalStateException should be thrown when task is in IDLE or DONE state
Java SE:
progress value (double):
- NaN = no progress
- +Inf = indetermite progress
- 0..1 = progress 0..100%
Pseudocode:
statusText = "Preparing..."
progressValue = Double.POSITIVE_INFINITY
...
statusText = "Working...
progressValue = 0.0
try {
loop ...
if(cancelled) {
statusText = "Cancelling..."
progressValue = Double.POSITIVE_INFINITY
...
return;
}
progressValue = 0.0 --- 1.0
... end loop
statusText = "Finishing..."
progressValue = Double.POSITIVE_INFINITY
...
} finally {
progressValue = Double.NaN
if(cancelled)
statusText = "CANCELLED"
else
statusText = "DONE"
}
I'm sorry I didn't study JavaFX api already.
Thank you guys that you made great progress on JavaFX. (I'm waiting for SceneBuilder and linux dev preview)
Martin
----- Original Message -----
From: "Tom Eugelink" <tbee at tbee.org>
To: -dev at openjdk.java.openjfxnet
Sent: Wednesday, January 4, 2012 7:29:52 AM GMT +01:00 Amsterdam / Berlin / Bern / Rome / Stockholm / Vienna
Subject: Re: Review of solution for Cancelled Tasks
I'm not sure if we should go as far as trying to "somewhat" fix faulty code, because formally the background task should check if it's cancelled. So IMHO RT-17932 is not a JavaFX bug, but a coder's bug.
The only drawback to the proposed solution I can see, is that you cannot actually set the progress anymore .So if you are checking isCancelled and then want the progress to show 100% (which is what I often do), you now suddenly get an exception.
If you decided to go this way, the only change to your solution could be to throw an checked exception, for example "TaskIsCancelled", which forces the coder to deal with that situation. But for one I do not like checked exceptions and secondly it would require an API change.
Personally I'd flag RT-17932 as "not a bug" and tell the owner of the issue to start checking isCancelled.
Tom
On 2012-01-04 05:05, Richard Bair wrote:
> http://javafx-jira.kenai.com/browse/RT-17932
>
> I have a proposed solution for this bug, if anybody has a few minutes to give it some thought that would be great. In a nutshell:
>
> - When a task is cancelled, the background thread might continue to operate. This is the way Java works (we can't safely kill a thread outright, so we have to wait for the thread to terminate)
> - The developer of a Task can check isCancelled from the background thread and self-terminate
> - The developer can use any API that throws an InterruptedException and use that to indicate that the thread has been cancelled
> - A "run-away" background thread that is supposed to be cancelled, can continue to update the text, message, and progress of a task, giving the appearance that it isn't cancelled!
> - A cancelled task might want to legitimately update the text, message, or progress in the case of cancellation (maybe the message becomes "I'm cancelling this request").
>
> In trying to solve this problem, I wanted a solution where the naive implementation still works as one would expect, and a well considered solution works perfectly. I think I have a solution.
>
> If you write a Task, and do nothing smart:
>
> new Task() {
> public Object call() throws Exception {
> for (int i=0; i<100; i++) {
> doSomething();
> }
> return null;
> }
> };
>
> In this case, if the task is cancelled, then it will just keep running in the background, but since the task returns nothing and never updates the progress or text or message, it appears to have been successfully cancelled to any code using the task. Here is another dumb implementation:
>
> new Task() {
> public Object call() throws Exception {
> for (int i=0; i<100; i++) {
> doSomething();
> updateProgress(i, 100);
> }
> return null;
> }
> };
>
> This one will update the progress, and exhibits the bug. My proposal is that calling updateProgress, updateMessage, or updateText from a background thread of a cancelled Task will result in an IllegalStateException. So in this case, the attempt to updateProgress after the task was cancelled will actually cause the exception and, in this case, actually terminate the background thread. Two birds, one stone.
>
> In this case...:
>
> new Task() {
> public Object call() throws Exception {
> for (int i=0; i<100; i++) {
> try {
> doSomething();
> updateProgress(i, 100);
> } catch (Exception e) { }
> }
> return null;
> }
> };
>
> ...we are not so lucky. Although the progress won't be updated on a cancelled task, it will keep running in the background. But no harm, no foul. If the developer was smart they'd do something like:
>
> new Task() {
> public Object call() throws Exception {
> for (int i=0; i<100; i++) {
> if (isCancelled()) break;
> try {
> doSomething();
> updateProgress(i, 100);
> } catch (Exception e) { }
> }
> return null;
> }
> };
>
> Anyway, it seems that this solution doesn't hurt the naive case, and in some instances helps it, and in any case for more sophisticated library developers you have the tools to handle cancellation correctly. Here are two ways to correctly update the text after the task has been cancelled:
>
> new Task() {
> public Object call() throws Exception {
> for (int i=0; i<100; i++) {
> if (isCancelled()) {
> Platform.runLater(new Runnable() {
> public void run() {
> updateMessage("Cancelled")
> }
> });
> break;
> }
> try {
> doSomething();
> updateProgress(i, 100);
> } catch (Exception e) { }
> }
> return null;
> }
> };
>
> new Task() {
> public Object call() throws Exception {
> for (int i=0; i<100; i++) {
> if (isCancelled()) break;
> try {
> doSomething();
> updateProgress(i, 100);
> } catch (Exception e) { }
> }
> return null;
> }
>
> @Override protected void cancelled() {
> updateMessage("Cancelled");
> }
> };
>
> I think this works fairly well. Anything I'm missing?
>
> Thanks!
> Richard
More information about the openjfx-dev
mailing list