AbstractExecutorService的invokeAll方法中isDone检查是否冗余？

Great question! Let's unpack this by looking at the actual implementation of invokeAll() in AbstractExecutorService and analyzing what the isDone() check contributes.

First, here's the relevant code snippet for context:

public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks)
        throws InterruptedException {
    if (tasks == null)
        throw new NullPointerException();
    ArrayList<Future<T>> futures = new ArrayList<>(tasks.size());
    boolean done = false;
    try {
        // Step 1: Submit all tasks
        for (Callable<T> t : tasks) {
            RunnableFuture<T> f = newTaskFor(t);
            futures.add(f);
            execute(f);
        }
        // Step 2: Wait for all tasks to complete
        for (Future<T> f : futures) {
            if (!f.isDone()) { // <-- The check in question
                try {
                    f.get();
                } catch (CancellationException ignore) {
                } catch (ExecutionException ignore) {
                }
            }
        }
        done = true;
        return futures;
    } finally {
        // Cancel remaining tasks if we exited early
        if (!done)
            for (Future<T> f : futures)
                f.cancel(true);
    }
}

Let's break down the purpose of the isDone() check vs. directly calling f.get() #

1. What f.get() does on its own:

   • If the task is already completed (successfully, canceled, or failed), get() returns immediately (or throws an exception that's caught and ignored here).
   • If the task is not completed, get() blocks until it finishes or the current thread is interrupted.

2. What the isDone() check adds:

   • It skips the try/catch block for tasks that are already done. But since the catch blocks ignore CancellationException and ExecutionException anyway, this doesn't change the behavior—those exceptions would be caught and discarded even if we called get() on a completed task.
   • For tasks that are already done, calling get() directly would have the same end result as skipping the block: the method moves on to the next future without any side effects.

So is the check redundant? #

Functionally speaking, yes—removing the isDone() check would not change the behavior of invokeAll(). The get() method inherently handles both completed and incomplete tasks, and the exception handling logic is already set up to ignore the exceptions that might be thrown for completed tasks.

That said, there might be minor, non-functional reasons for including the check:

• Readability: It makes the intent clearer—we're explicitly waiting only for tasks that haven't finished yet.
• Tiny performance optimization: Skipping the try/catch block for completed tasks might save a negligible amount of overhead, though modern JVMs optimize exception handling so much that this is unlikely to be noticeable in practice.

Final takeaway #

You could safely remove the isDone() check without breaking any functionality. The core logic of waiting for all tasks to complete is entirely handled by f.get() itself.

内容的提问来源于stack exchange，提问作者Gaurav