concurrency using ExecutorService

Question

I have this simple program to count numbers from 1 to 9 using ThreadPool and ExecutorService. Each Thread is waiting for 1 sec to execute. However, below program gives me random output for each execution.

How do I fix this so that it always produces 45?

public static void main(String[] args) throws InterruptedException {
    AtomicLong count = new AtomicLong(0);
    ExecutorService executor = Executors.newFixedThreadPool(10);
    List<Integer> list = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9);
    for(Integer i : list) {
        executor.execute(new Runnable() {
            @Override
            public void run() {
                try {
                    Thread.sleep(1000);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                count.set(count.get() + i);
            }
        });
    }

    System.out.println("Waiting...");

    executor.shutdown();
    executor.awaitTermination(Long.MAX_VALUE, TimeUnit.MINUTES);
    System.out.println("Total::"+count.get());

    System.out.println("Done");
}

Answer 1

Instead of

count.set(count.get() + i);

use

count.addAndGet(i);

Method addAndGet adds value atomically but sequential get and set is not atomic operation.

Answer 2

AtomicLong has special methods which are atomic. You only get atomic guarantees when using them (separate calls to add() and get() will not be atomic). There are methods on AtomicLong which will "add" to the current value in an atomic fashion.