I am enhancing an existing algorithm that consists of multiple independent steps to use concurrent tasks. Each of the tasks will create multiple objects to hold its results. In the end, I would like to have a list of all the results to return from the controlling method. At the moment, my code looks something like that
private final ExecutorService pool = ...;
// A single task to be performed concurrently with other tasks.
private class WorkHorse implements Callable<Void> {
private final Collection<X> collect;
public WorkHorse(Collection<X> collect, ...) {
this.collect = collect;
}
public Void call() {
for (...) {
// do work
synchronized (this.collect) {
this.collect.add(result);
}
}
return null;
}
}
// Uses multiple concurrent tasks to compute its result list.
public Collection<X> getResults() {
// this list is supposed to hold the results
final Collection<X> collect = new LinkedList<X>();
final List<WorkHorse> tasks = Arrays.asList(
new WorkHorse(collect, ...), new WorkHorse(collect, ...), ...);
this.pool.invokeAll(tasks);
// ## A ##
synchronized (collect) {
return collect;
}
}
Do I actually need the synchronized at "## A ##" to enforce a happens-before relationship with the modifying operations in the worker tasks? Or can I rely on all write operations to have happened after invokeAll returns and be visible to the controlling thread? And is there any reason, why I should not return the results collection from within its own synchronized block?