Multiple threads and AtomicInteger

Question

I took this code from a tutorial on atomics, it stated :-

"By using AtomicInteger as a replacement for Integer we're able to increment the number concurrently in a thread-safe manor without synchronizing the access to the variable. The method incrementAndGet() is an atomic operation so we can safely call this method from multiple threads."

It said this would return the correct result, 1000 however neither instance reaches 1000, they are usually considerably less, e.g.

test1 result = 532
test2 result = 128

Whats wrong ?

public class AtomicsTest {

    public static void main(String... args){

        AtomicsTest test = new AtomicsTest();        
        test.test1();        
        test.test2();                             
    }

    public void test1() {
        AtomicInteger atomicInt = new AtomicInteger(0);

        ExecutorService executor = Executors.newSingleThreadExecutor();                
        IntStream.range(0,1000).forEach(i->executor.submit( atomicInt::incrementAndGet ));
        System.out.println("test1 result = "+ atomicInt.get());
        executor.shutdown();
    }

    public void test2() {

        AtomicInteger atomicInt = new AtomicInteger(0);

        ExecutorService executor = Executors.newFixedThreadPool(2);            
        IntStream.range(0,1000).forEach(i->executor.submit( atomicInt::incrementAndGet ));
        System.out.println("test2 result = " + atomicInt.get());        
        executor.shutdown();           
    }    
}

Apart from the answer provided, your example does not really test synchronisation of the atomic variable. You are using a single-threaded executor to increment your variable, so even with a non-atomic variable you would increment it one at a time and get 1000 as a result as there is only 1 thread accessing the variable at any time. — pandaadb, Mar 29 '17 at 16:18
@pandaadb that may be true of his first test, but not the second. I didn't spot it immediately but the only difference is the executor. — Michael, Mar 29 '17 at 21:50
@Michael you are absolutely right. I overlooked the second one but saw the first :) I thought it worth pointing out — pandaadb, Mar 30 '17 at 08:21

Michael · Accepted Answer · 2017-03-29T16:08:25.940

Your issue is that you immediately print the value after having submitted the threads, despite not all threads necessarily having completed.

public void test1() {
    AtomicInteger atomicInt = new AtomicInteger(0);

    ExecutorService executor = Executors.newSingleThreadExecutor();                
    IntStream.range(0,1000).forEach(i->executor.submit( atomicInt::incrementAndGet ));

    // All threads submitted (not necessarily finished)

    System.out.println("test1 result = "+ atomicInt.get());

    executor.shutdown();

    // Threads are still not necessarily done
}

If you explicitly wait for the executor service to finish before you print the value you should see the result you expect:

public void test1() {
    try {
        AtomicInteger atomicInt = new AtomicInteger(0);

        ExecutorService executor = Executors.newSingleThreadExecutor();
        IntStream.range(0,1000).forEach(i->executor.submit( atomicInt::incrementAndGet ));

        executor.shutdown();
        // Wait a maximum of ~a million billion years
        executor.awaitTermination(Long.MAX_VALUE, TimeUnit.HOURS);

        System.out.println("test1 result = "+ atomicInt.get());
    }
    catch (InterruptedException ex)
    {
        // Oh no!
    }
}

I tend to wait for executors Long.MAX_VALUE as a better pattern. — Gray, Mar 29 '17 at 16:06
Good point. I've edited it in. I like the idea that it might be waiting literally a million billion years. — Michael, Mar 29 '17 at 16:09

Multiple threads and AtomicInteger

1 Answers1