Strange concurrent code behavior

Question

I'm currently in the process of learning Java concurrency. And I am very surprised by the way following code behaves.

import java.util.concurrent.*;

public class Exercise {
    static int counter = 0;

    static synchronized int getAndIncrement() {
        return counter++;
    }

    static class Improper implements Runnable {

        @Override
        public void run() {
            for (int i = 0; i < 300; i++) {
                getAndIncrement();
            }
        }
    }


    public static void main(String[] args) {
        ExecutorService executorService = Executors.newFixedThreadPool(3);
        for (int i = 0; i < 300; i++) {
            executorService.submit(new Improper());
        }
        executorService.shutdown();
        System.out.println(counter);
    }
}

Shouldn't it output 90000 all the time? Instead the result differs all the time.

Answer 1

1. executorService.shutdown() doesn't wait for the service to shut down. You need a call to awaitTermination.

2. you access the counter from the main method without locking. I think you would narrowly escape a data race if you waited for the executor service to shut down, but be warned that, in general, you must synchronize on all accesses of a shared variable, not just writes, to have any visibility guarantees from the Java Memory Model.