1

I have 2 class. First class is controller. When you click on the enter button, in the collection adding many objects in different threads and presented this collection as the class of the controller when the loop exits after 10 iterations.

public class Controller {
    private int count = 3;

    @FXML
    private Button enter;

    @FXML
    public void getData() throws IOException{
        //Create collection
        List<String> synchList = Collections.synchronizedList(new ArrayList<String>());

        ExecutorService executor = Executors.newFixedThreadPool(count);

        for (int i=0; i<10; i++) {

            Runnable myThread= new MyThread(synchList); 
            executor.execute(myThread);

        }

        executor.shutdown();

        System.out.println(synchList); //Show collection
}

The second class is MyThread. It performs some task. The received object is stored in collection early (for example, add 3 of the object with a delay, simulating the performance of some task)

public class MyThread implements Runnable{
    public List<String> list = null; 

    public MyThread(List<String> list){
        this.list = list;   
    }

    @Override
    public void run() {
        try {
            Thread.sleep(2000);
            list.add("Object2");
            Thread.sleep(2000);
            list.add("Object3");
            Thread.sleep(2000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
}

At the end of the code execution, I get an empty list. Please tell me what am I doing wrong and how to fix this situation? Thank you!

Tomas
  • 1,567
  • 3
  • 21
  • 38
  • Possible duplicate of [How to make a Java thread wait for another thread's output?](http://stackoverflow.com/questions/289434/how-to-make-a-java-thread-wait-for-another-threads-output) – Itai Nov 23 '16 at 15:12
  • It's not duplicate, because this does not apply to my problem. – Tomas Nov 23 '16 at 16:20

2 Answers2

2

Take a look at the documentation of ExecutorService.shutdown:

Initiates an orderly shutdown in which previously submitted tasks are executed, but no new tasks will be accepted. Invocation has no additional effect if already shut down.

This method does not wait for previously submitted tasks to complete execution.

Using shutdown only prevents new tasks from being submitted. It does not make sure all tasks are finished.

Instead you could use awaitTermination to achieve the desired effect:

executor.shutdown();

while (true) {
    try {
        if (executor.awaitTermination(1, TimeUnit.HOURS)) {
            break;
        }
    } catch (InterruptedException ex) {
    }
}

System.out.println(synchList); //Show collection

You should however make sure the code isn't run from the application thread, since otherwise the GUI becomes unresponsive...

Update

In case you want the results to be added sequentially, implementing Callback<List<String>> with MyThread and using the results would be the better option, since this allows you to retrieve results from the tasks (sublists of the final list) and combine the results to a single list:

public class MyThread implements Callable<List<String>> {

    @Override
    public List<String> call() throws Exception {
        List<String> list = new ArrayList(2);
        Thread.sleep(2000);
        list.add("Object2");
        Thread.sleep(2000);
        list.add("Object3");
        Thread.sleep(2000);
        return list;
    }
}
//Create collection
List<String> resultList = new ArrayList<>();

ExecutorService executor = Executors.newFixedThreadPool(count);

Callable<List<String>>[] tasks = new Callable[10];
for (int i = 0; i < tasks.length; i++) {
    tasks[i] = new MyThread();
}

List<Future<List<String>>> futures = executor.invokeAll(Arrays.asList(tasks));
executor.shutdown();

for (Future<List<String>> future : futures) {
    // combine results
    resultList.addAll(future.get());
}

System.out.println(resultList); //Show collection
Community
  • 1
  • 1
fabian
  • 80,457
  • 12
  • 86
  • 114
  • Oh, thanks! It's work! But, what can i use if I want objects in the collection were added sequentially? – Tomas Nov 23 '16 at 16:19
  • 1
    @Tomas In this case it is indeed better to use `Future` (but also `Callable>`), since this is exactly what `Future` is supposed to be used for. See my edit. – fabian Nov 23 '16 at 16:57
  • thanks. It's good answer! @hoaz wrote about `Future` in her example too, – Tomas Nov 23 '16 at 17:08
1

You do not wait for task completion and shutdown executor before tasks even start. Try following:

List<Future<?>> results = new ArrayList<>();
for (int i = 0; i < 10; i++) {
    Runnable myThread = new MyThread(synchList); 
    results.add(executor.submit(myThread));
}

// wait loop
for (Future<?> result : results) {
    result.get();
}

executor.shutdown();
hoaz
  • 9,883
  • 4
  • 42
  • 53
  • Thanks! But, you mean that I need to add another to the collection List> results ? p.s. results.add() don't work. – Tomas Nov 23 '16 at 15:54
  • Yes, results.add should work fine with this method: `Future> submit(Runnable task)` – hoaz Nov 23 '16 at 16:07