Wait for every Future in a List<Future> to be done [closed]

Question

I call a method that returns a Future, once for each element in a List<Principal>, so I end up with a List<Future<UserRecord>>.

The method returning Future is library code and I have no control over how that code gets run, all I have is the Future.

I want to wait for all the Futures to finish (success or failure) before proceeding further.

Is there a better way to do so than this:

List<Principal> users = new ArrayList<>();
// Fill users
List<Future<UserRecord>> futures = getAllTheFutures(users);
List<UserRecord> results = new ArrayList<>(futures.size());
boolean[] taskCompleted = new boolean[futures.size()];
for (int j = 0; j < taskCompleted.length; j++) {
    taskCompleted[j] = false;
}
do {
    for (int i = 0; i < futures.size(); i++) {
        if (!taskCompleted[i]) {
            try {
                results.add(i, futures.get(i).get(20, TimeUnit.MILLISECONDS));
                taskCompleted[i] = true;
            } catch (TimeoutException e) {
                // Do nothing
            } catch (InterruptedException | ExecutionException e) {
                // Handle appropriately, then...
                taskCompleted[i] = true;
            }
        }
    }
} while (allNotCompleted(taskCompleted));

For the curious:

private boolean allNotCompleted(boolean[] completed) {
    for (boolean b : completed) {
        if (!b)
            return true;
    }
    return false;
}

Unlike in this answer to Waiting on a list of Future I don't have control over the code that creates the Futures.

Feathercut

Does allNotCompleted(taskCompleted) snippet check entire array and return an overall result ? – Moule

Yes, of course. – Feathercut

Why are you using a timeout on get()? What purpose does it serve since you'll keep looping back to it. Also, what does "Handle appropriately" mean in your context, as it affects a lot on what the code needs to do and can do. You're new to Java aren't you? You're initializing your boolean[] to false, even though it's the default. – Tasker

@Tasker In my case "Handle appropriately" means let the results list contain a null for element i as that means failure to get a valid result. In other places in my code "Handle appropriately" may mean something different. Don't see how that is relevant to waiting for all Futures to finish (one way or another). Yeah, I guess I could do without a timeout on get(). – Feathercut

Why is this so complicated? What's wrong with for (Future<UserRecord> future : futures) results.add(future.get());? – Douzepers

@Douzepers Maybe because I am simply over-thinking things :-). – Feathercut

@Elysiumplain the method returning the futures is external, there's no need or use for adding your own executors. – Tasker

Answer 1

Your code can be simplified a lot. An equivalent version could be written as follows, unless you have requirements that you didn't specify in the question.

List<Principal> users = // fill users
List<Future<UserRecord>> futures = getAllTheFutures(users);
List<UserRecord> results = new ArrayList<>();

for (int i = 0; i < futures.size(); i++) {
        try {
            results.add(futures.get(i).get());
        } catch (InterruptedException | ExecutionException e) {
            // Handle appropriately, results.add(null) or just leave it out
        }
    }
}

Tasker

So the only thing you have removed is the wait in the get. Makes sense. One thing you missed out on is that the result of the ith future gets added at the ith index in results. All the snide remarks were unnecessary. But maybe that's just how you get your jollies. – Feathercut

@Feathercut I don't really see why you'd care about the index. Maybe you want to keep a separate List<Principal> failed where you add the principals for the futures that threw an exception with failed.add(users.get(i)). I'm trying to provide good answers, and it's hard to do that when there are people who provide bad answers (and people who upvote those bad answers because they don't understand that they're bad). – Tasker

Things are being generated in order and it's just easier to keep everything in order down the chain. I appreciate your simple code and your pointing out that the wait was unnecessary. I can understand the good & the bad once I see the code. If I was confident in my solution I wouldn't have asked for help here. – Feathercut

@markvgti: the for loop iterates over the element in order and add will append at the current end of the list, so the resulting list will be in the same order as the source list. In fact, even this answer’s code is more complicated than necessary. An even simpler for (Future<UserRecord> future : futures) try { results.add(future); } catch (…) { …} would do as well. No need to deal with indices. – Chickamauga

Answer 2

You could simply do a reductive list; removing successful responses from your list and iterating until empty.

List<Principal> users = // fill users
List<Future<UserRecord>> futures = getAllTheFutures(users);
List<UserRecord> results = new ArrayList<>();

for (int i = 0; i < futures.size(); i++) {
        try {
            results.add(futures.get(i).get(<how long you want before your application throws exception>));

        }
        catch (InterruptedException | ExecutionException e) {
            // Handle appropriately, results.add(null) or just leave it out
        }
        catch (TimeoutException timeoutEx) {
            // If the Future retrieval timed out you can handle here
        }

    }
}

Since your intent is to collect a "set" of Jobs before progressing, waiting until you get a return on thread index X in this case will give a time cost of (roughly) the last returned thread.

Or, if you plan to abort all threads in the set if any fail, you can use Java 8 CompletableFuture

CompletableFuture[] cfs = futures.toArray(new CompletableFuture[futures.size()]);

    return CompletableFuture.allOf(cfs)
            .thenApply(() -> futures.stream()
                                    .map(CompletableFuture::join)
                                    .collect(Collectors.toList())
            );

• credit to Kayaman for simplified code base.

Colubrid

No, the intent was to collect all. It was just implemented in a weird and confusing way in the question. – Tasker

Your code is equivalent to mine, except yours doesn't do what the title asks, i.e. "Wait for every Future*. Come on people, it's not that hard. – Tasker

So we're agreed this question basically serves no purpose to begin with since Future's documentation explains how, and has implemented functions to do this. Should close question maybe as opinionated (aside from obvious simplification of poster's example code)? – Colubrid

I don't see any issue of opinion here. The question asks for a "better way to do this", and I answered with a better, idiomatic way. The other answer is just horrible, and your answer started out as wrong, but now you included CompletableFuture for no real reason. – Tasker