Properly cancel JavaFX Task that launches ExecutorService

Question

I am attempting to write a GUI application that performs many computationally intensive tasks. Because these tasks take some time, I would like to run them on multiple threads using an ExecutorService. However, waiting for these tasks to finish would freeze the UI, so I run that as a Task inside its own thread, which updates the UI on the progress of the ExecuterService. The user starts the tasks using the Start button, and should be able to cancel them with Cancel.

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.Future;
import java.util.concurrent.Executors;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ExecutionException;

import javafx.application.Application;
import javafx.concurrent.Task;
import javafx.geometry.Insets;
import javafx.scene.layout.HBox;
import javafx.scene.control.Button;
import javafx.scene.Scene;
import javafx.stage.Stage;

class DoWorkTask extends Task<List<Double>> {

    // List of tasks to do in parallel
    private final List<Callable<Double>> tasks;

    // Initialize the tasks
    public DoWorkTask(final int numTasks) {

        this.tasks = new ArrayList<>();

        for (int i = 0; i < numTasks; ++i) {
            final int num = i;
            final Callable<Double> task = () -> {
                System.out.println("task " + num + " started");
                return longFunction();
            };

            this.tasks.add(task);
        }
    }

    @Override
    protected List<Double> call() {

        final ExecutorService executor = Executors.newFixedThreadPool(4);

        // Submit all tasks to the ExecutorService
        final List<Future<Double>> futures = new ArrayList<>();
        this.tasks.forEach(task -> futures.add(executor.submit(task)));

        final List<Double> result = new ArrayList<>();

        // Calling task.cancel() breaks out of this
        // function without completing the loop
        for (int i = 0; i < futures.size(); ++i) {
            System.out.println("Checking future " + i);

            final Future<Double> future = futures.get(i);

            if (this.isCancelled()) {
                // This code is never run
                System.out.println("Cancelling future " + i);
                future.cancel(false);
            } else {
                try {
                    final Double sum = future.get();
                    result.add(sum);
                } catch (InterruptedException | ExecutionException e) {
                    throw new RuntimeException(e);
                }
            }
        }

        executor.shutdown();
        return result;
    }

    // Some computationally intensive function
    private static Double longFunction() {

        double sum = 0;
        for (int i = 0; i < 10000000; ++i) {
            sum += Math.sqrt(i);
        }

        return sum;
    }

}

public class Example extends Application {

    final Button btnStart = new Button("Start");
    final Button btnCancel = new Button("Cancel");
    final HBox box = new HBox(10, btnStart, btnCancel);
    final Scene scene = new Scene(box);

    @Override
    public void start(final Stage stage) {

        box.setPadding(new Insets(10));

        btnStart.setOnAction(event -> {

            final DoWorkTask task = new DoWorkTask(100);

            btnCancel.setOnAction(e -> task.cancel());

            task.setOnSucceeded(e -> System.out.println("Succeeded"));

            task.setOnCancelled(e -> System.out.println("Cancelled"));

            task.setOnFailed(e -> {
                System.out.println("Failed");
                throw new RuntimeException(task.getException());
            });

            new Thread(task).start();
        });

        stage.setScene(scene);
        stage.show();
    }
}

However, upon starting the tasks and pressing the Cancel button, the call() function seems to end immediately without iterating over the rest of the futures. Some sample output.

task 0 started
task 1 started
task 2 started
Checking future 0
task 3 started
Checking future 1
task 4 started
Checking future 2
task 5 started
Cancelled
// Should continue to print Checking future x and
// Cancelling future x, but doesn't
task 6 started
task 7 started
task 8 started
task 9 started
...

None of the remaining futures are cancelled, and it seems they are not even iterated over; the call() function ends immediately. When the callables are not run inside an ExecutorService, but just sequentially inside the DoWorkTask, this problem doesn't occur. I am rather puzzled.

Answer 1

Your problem is here:

try {
    final Double sum = future.get();
    result.add(sum);
} catch (InterruptedException | ExecutionException e) {
    throw new RuntimeException(e);
}

When you click on the button cancel, it actually cancels the main task DoWorkTask which interrupts the thread that executes the main task so as it is waiting for a result with future.get(), an InterruptedException is raised but with your current code you then throw a RuntimeException such that your main task exit immediately and thus cannot interrupt the sub tasks, you should continue when a InterruptedException is thrown.

try {
    final Double sum = future.get();
    result.add(sum);
} catch (ExecutionException e) {
    throw new RuntimeException(e);
} catch (InterruptedException e) {
    // log something here to indicate that the task has been interrupted.
}