Do non-synchronized i++ overlap?

Question

Attempting to get Java multithreading basics I've run into a case I can't understand. Community, please share your exp why this is happening: I have a Runnable:

class ImproperStateWorker implements Runnable {
    private int val = 0;
    @Override
    public void run() {
        //Where are missing ticks?
        for (int i = 0; i < 1000; i++) {
                    val++;
                    Thread.yield();
                    val++;
        }
        showDataState();
    }

    public void showDataState() {
        System.out.print(Thread.currentThread() + " ; ");
        System.out.println(val);
    }
}

which is started via:

public class ImproperState {
public static void main(String[] args) {
    ImproperStateWorker worker = new ImproperStateWorker();
    for (int i = 0; i < 2; i++) {
        Thread thread = new Thread(worker);
        thread.start();
    }
}

}

I understand that the idea is to make both increment operations atomic by using synchronized() {...}, and so on. But I am confused with the following: Why running these two Runnables (without sync) does not produce the consistent result of 4000 (1000 x 2 increments per task)? Regardless of how the context is switched between the two task, I expect that the tasks will perform 2000 increments each, I don't care what the order will be.

However, the program output gives ~3.5K. The only idea I can think of is that "missing" increments emerge because some of them are made simultaneously so that val++ called from two threads actually increases the value by 1. But this is a very vague assumption. Thanks for sharing your experience.

Answer 1

You have a race condition in your code. Consider the following possible interleaving:

1. Thread 1: reads val → 0
2. Thread 1: increments 0 → 1
3. Thread 1: writes val ← 1
4. Thread 1: reads val → 1
5. Thread 1: increments 1 → 2
6. Thread 1: writes val ← 2
7. Thread 2: reads val → 2
8. Thread 2: increments 2 → 3
9. Thread 2: writes val ← 3
10. Thread 2: reads val → 3
11. Thread 2: increments 3 → 4
12. Thread 2: writes val ← 4
13. End State: val == 4

All is good. But, consider this equally possible interleaving:

1. Thread 1: reads val → 0
2. Thread 2: reads val → 0
3. Thread 1: increments 0 → 1
4. Thread 2: increments 0 → 1
5. Thread 1: writes val ← 1
6. Thread 2: writes val ← 1
7. Thread 1: reads val → 1
8. Thread 2: reads val → 1
9. Thread 1: increments 1 → 2
10. Thread 2: increments 1 → 2
11. Thread 1: writes val ← 2
12. Thread 2: writes val ← 2
13. End State: val == 2

Oops!

In the code as written in your question, the result can be anything between 2000 and 4000.

One way to fix this is to use an AtomicInteger with AtomicInteger.getAndIncrement() or AtomicInteger.incrementAndGet() (in your case it doesn't matter which since you ignore the return value; the correct equivalent to postfix val++ would be val.getAndIncrement()) instead, like this (you only need to change three places, not counting the import.):

import java.util.concurrent.atomic.AtomicInteger;

class FixedStateWorker implements Runnable {
    private AtomicInteger val = new AtomicInteger();
    //      ↑↑↑↑↑↑↑↑↑↑↑↑↑       ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑

    @Override
    public void run() {
        for (int i = 0; i < 1000; i++) {
            val.getAndIncrement();
            //  ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑
            Thread.yield();
            val.getAndIncrement();
            //  ↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑↑
        }
        showDataState();
    }
}