Wait for every Future in a List to be done

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.

Does `allNotCompleted(taskCompleted)` snippet check entire array and return an overall result ? — Sachith Dickwella, Dec 14 '17 at 06:24
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. — Kayaman, Dec 14 '17 at 06:36
@Kayaman 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()`. — markvgti, Dec 14 '17 at 06:42
Why is this so complicated? What's wrong with `for (Future future : futures) results.add(future.get());`? — shmosel, Dec 14 '17 at 06:43
@shmosel Maybe because I am simply over-thinking things :-). — markvgti, Dec 14 '17 at 06:44
@Elysiumplain the method returning the futures is external, there's no need or use for adding your own executors. — Kayaman, Dec 14 '17 at 06:49

score 5 · Accepted Answer · edited Jan 27 '20 at 23:37

5

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
        }
    }
}

edited Jan 27 '20 at 23:37

jcsahnwaldt Reinstate Monica

3,434
4
35
33

answered Dec 14 '17 at 06:41

Kayaman

72,141
5
83
121

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 `i`th future gets added at the `i`th index in `results`. All the snide remarks were unnecessary. But maybe that's just how you get your jollies. – markvgti Dec 14 '17 at 07:18
@markvgti I don't really see why you'd care about the index. Maybe you want to keep a separate `List 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). – Kayaman Dec 14 '17 at 07:36
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. – markvgti Dec 14 '17 at 08:55
3

@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 future : futures) try { results.add(future); } catch (…) { …}` would do as well. No need to deal with indices. – Holger Dec 14 '17 at 10:53

Elysiumplain · Answer 2 · 2017-12-14T07:19:12.457

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.

No, the intent was to collect all. It was just implemented in a weird and confusing way in the question. — Kayaman, Dec 14 '17 at 07:06
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. — Kayaman, Dec 14 '17 at 07:11
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)? — Elysiumplain, Dec 14 '17 at 07:22
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. — Kayaman, Dec 14 '17 at 07:32

Wait for every Future in a List to be done

2 Answers2