Stopping all Runnable tasks when one is terminated

I want to run couple of Runnables. If one is failed, all others' execution should be terminated. Here is my approach. I got idea on this from this question and this article.

In my code, I check the runnable which completes its task first among all and evaluate it. If that checking is failed (completionService.take().get()), all remaining executions are cancelled.

```
java.util.List tasks = Arrays.asList("1", "2", "3", "4", "5");
List futureList = new ArrayList();
final ExecutorService pool = Executors.newFixedThreadPool(5);
final ExecutorCompletionService completionService = new ExecutorCompletionService<>(
pool);
for (final String site : tasks) {

futureList.add(completionService.submit(new Callable() {
@Override
public String call() throws Exception {
int i = Integer.parseInt(site);
System.out.println("iterating i: " + i);
if (i == 4) {
throw new RuntimeException("Forcd exception");
}

if (i % 2 == 0) {
try {
Thread.sleep(2000); // simply wanted to change the
// execution time...this is for
// 2s and others 5s.
} catch (InterruptedException e) {
System.err.println("Interrupted sleeping: "
+ e.getCause());
}
} else {
try {
Thread.sleep(5000);
} catch (InterruptedException e) {
System.err.println("Interrupted sleeping: "
+ e.getCause());
}
}
return site;
}
}));
}

for (Future ft : futureList) {

if (!futureList.isEmpty()) {
try {
S

Your general approach is sound

When your Callable throws, the throw will be propagated out of the Future, you catch it and cancel all remaining futures. I believe it is safe to cancel a completed job so you might not need the !isdone() check. But that's nitpicking.
Then you shutdown your thread pool to clean up.

Your implementation is a bit messy

The loop where you iterate over all the futures seems a bit messy to me.

For example:

for (Future ft : futureList) {
    if (!futureList.isEmpty()) {

this condition will always be true. And you are never removing from the list (if you were, you would get a ConcurrentModificationException. So the check is wholly redundant.

So you loop over all of the futures in futureList and call take() on your completionService. This doesn't make sense to me, if you're going to call take() use a loop that describes what you want to do better. This is further strengthened by the fact that you never use the ft loop variable.

Maybe something like this:

while(futureList.size() > 0) {
    Future ft = completionService.take();
    futureList.remove(ft);

    try{
        String task = ft.get();
    }catch(InterruptedException e){
        // Interrupted
    }catch(ExecutionException e){
        // Snap, something went wrong in the task! Abort! Abort! Abort!
        for(Future f : futureList){
            f.cancel(true);
        }
        futureList.clear();
    }   
}
pool.shutdownNow();

StackExchange Code Review Q#93589, answer score: 4