Race condition even after synchronising

Question

I am trying to run multiple threads. I am clearly getting race condition and able to resolve it as follows:

final Data data = new Data();
for (int i = 0; i < numberOfThreads; i++) {
    final Thread thread = new Thread(new Runnable() {
        @Override
        public void run() {
            //using this sync block to stop the race condition 
            synchronized (data){
                final int value = data.getValue();
                data.setValue(value + 1);
            }
        }
    });
    thread.start();
}

But I do not want to sync on this block and instead want to handle it at the Data class. So I removed the above sync block and instead synced the get and set methods over at the Data class as follows but this still causes race conditions. Why the issue even though I have synced them?

public class Data {

    private int value;

    public synchronized int getValue(){
        return this.value;
    }

    public synchronized void setValue(int num){
        this.value = num;
    }
}

Answer 1

Because you didn't. You synced either of them, so two threads cannot execute the methods at the same time, but one thread can execute getValue() then finishes getValue() and before it enters setValue() another thread gets its turn and calls getValue() which is perfectly legal and your race condition.

Btw. just in case Data would be your whole class, AtomicInteger would be the same, but done properly. There you e. g. have an incrementAndGet() method that does the read and write operation in one synchronisation block which is the essential point in your case.

Answer 2

Adding synchronized to the individual methods is similar to doing something like this

final Data data = new Data();
for (int i = 0; i < numberOfThreads; i++) {
    final Thread thread = new Thread(new Runnable() {
        @Override
        public void run() {
            synchronized (data){
                final int value = data.getValue();
            }
            synchronized (data){
                data.setValue(value + 1);
            }
        }
    });
    thread.start();
}

Where a thread could very clearly get stuck between the get and the set. To resolve this, you either need to add a new synchronized method to the Data class that accomplishes the value + 1 task, or wrap both lines in a synchronized block, as you have done in your code.