Incrementing AtomicInteger in Java in 1000 threads does not generate value 1000

Question

I am executing a java code where I have an AtomicInteger on which 1000 threads are trying to perform an incrementAndGet(). I was expecting the final value to be 1000. But each run is generating all sorts of different values. The code is as follows :

class task implements Runnable {
    AtomicInteger i ;
    task(AtomicInteger ai) {i =ai ;}
    public void run() { i.incrementAndGet() ; }
}

class jlt {
    public static void main(String[] args) throws Exception {
        AtomicInteger atomicInt = new AtomicInteger(0);

        ExecutorService executor = Executors.newFixedThreadPool(2);
        for(int i=1; i<=1000; i++)
            executor.submit(new task(atomicInt)) ;

        executor.shutdown() ;

        System.out.println(atomicInt.get()); // 1000 is expected 
    }
}

I am not able to figure out the mistake that I am making here or in my understanding

Unrelated: A) read about java naming conventions - class names go UpperCase B) always always use braces for loop/if/... blocks. Even when there is just one line. It is so easy to get this wrong. — GhostCat, Oct 23 '17 at 12:48
Also note that you only have 2 threads incrementing the counter, not 1000. — JB Nizet, Oct 23 '17 at 12:49
And the real answer here: don't assume what some library method does. Read its javadoc. It is very clearly documented what `shutdown()` is doing. — GhostCat, Oct 23 '17 at 12:50
I wanted to add that `submit()` will return a `Future` which you could add to a list. Once you've submitted all operations you can loop through the list of Futures calling `get()` which will block until it's completed running — fbailey, Nov 19 '20 at 18:56

score 10 · Answer 1 · answered Oct 23 '17 at 12:47

10

You're not waiting for the threads to finish before printing out the number.

Add a executor.awaitTermination(10, TimeUnit.SECONDS) after shutdown() and you might see something that you expected.

answered Oct 23 '17 at 12:47

Kayaman

72,141
5
83
121

1

Or read that duplicated question ;-) – GhostCat Oct 23 '17 at 12:49

score 3 · Accepted Answer · answered Oct 23 '17 at 12:47

From https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/ExecutorService.html#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. Use awaitTermination to do that.

That means that when you call atomicInt.get() not all threads have been executed and some had no chance of incrementing the AtomicInteger.

Incrementing AtomicInteger in Java in 1000 threads does not generate value 1000

2 Answers2

Linked

Related