Java list collect results from different threads in list

Question

I have several computing threads that create result values (Objects). After each thread is finished a 'add' method from a result collector is called. This result collector is singleton, so there is only one representation.

Inside the result collector is a list which holds result objects:

List<TestResult> results = Collections.synchronizedList(new ArrayList<>());

The add method adds the result of each thread to the list:

  public void addResult(TestResult result){
        System.out.println("added " + result.getpValue());
        this.results.add(result);
    }

It is called within the thread, after the computing stuff is done.

The big problem is: After all threads are finished the list of results is empty. As you can see in the addResult method I added a print statement for the pValue. The p value of all results is printed out.

So it looks like the threads work on different lists. Despite the fact that the collector class is singleton.

It was asked for the complete code of the result collector (Javadoc removed to trim size)

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;


public class ResultCollector {

    private static ResultCollector resultCollector;
    private final List<TestResult> results;

    public static ResultCollector getInstance(){
        if(resultCollector == null){
            resultCollector = new ResultCollector();
        }

        return resultCollector;
    }

    private ResultCollector() {
        results = Collections.synchronizedList(new ArrayList<>());
    }

    public void addResult(TestResult result){
        System.out.println("added " + result.getpValue());
        this.results.add(result);
    }
}

I updated the add method to print out the hash of the current class to make sure every thread has the same with: System.out.println("added to" + System.identityHashCode(this) + " count: " +results.size());

The output hash code is the same for all threads and the size increases to the expected value. Also the hash code is the same when I call my toString method or getter for the list outside the multithread environment.

Calling of the threads:

public IntersectMultithread(...) {

    Set<String> tracks = intervals.keySet();

    for(String track: tracks){

        IntersectWrapper wrapper = new IntersectWrapper(...);
        wrappers.add(wrapper);
        exe.execute(wrapper);
    }

    exe.shutdown();
}

Answer 1

A Synchronized list is just a wrapper over a list. You should actually be using Concurrent Collections for this purpose in modern Java; they implement smarter and more efficient locking and provide better performance.

Caveat: the only synchronized list is one that copies on write. So, if that's an issue (i.e. you're adding more than iterating), then your way is fine).*

The error in your code is almost certainly in your singleton class which you haven't shown. Either your class is not truly a singleton (did you use an enum? that's a good way to guarantee it), or your list creation is more confusing than let on.

If you post more code, I can update the answer with more info :).

---

EDIT: I think your problem is here based on your updated code:

exe.shutdown();

You need to wait for the executor to complete with awaitTermination() with a good timeout relevant to the work you are doing.

Your threads just start and die off instantly right now :)

For example:

taskExecutor.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);

From here: https://stackoverflow.com/a/1250655/857994

Answer 2

Addition to the correct answer above

Yes, the exe.shutdown(); is the problem, but the threads do not die instantly, instead they seem to run through. This is why the 'add' method printed everything correct if extended with a print.

The issue was that my output was done before the threads could finish their computation. So there were no values at that time, shortly after the threads finish and the print works.