Why this code is not thread-safe?

Question

import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.IntStream;

public class Synchronized{
    public static AtomicInteger count = new AtomicInteger(0);
    public static synchronized void increment() {
        count.incrementAndGet();
    }
    public static void main(String[] args) {
        ExecutorService exector = Executors.newFixedThreadPool(2);
        IntStream.range(0, 1000).forEach( i -> exector.submit(Synchronized:: increment));
        System.out.println(count.get());    
    }
}

This does not print 1000 at the end. It prints anywhere from 600 to 1000 . so It appears that it is not thread-safe. Could someone explain why this is not thread safe?

You're not waiting for the executors to finish their job until printing out the result — jontro, Oct 30 '17 at 22:10

score 2 · Answer 1 · answered Oct 30 '17 at 22:12

You are not waiting for the execution of your tasks. Your stream is handing of the tasks to the pool, and then immediately prints out the current (i.e. not final) value of your counter. The counter increments properly, but you ask for its value too early. You would have to wait for the executor to be done before your sysout.

Read this answer about the use of shutdown() and awaitTermination().

Why this code is not thread-safe?

1 Answers1