Java volatile variable doesn't behave correctly.

Question

public class MyThread
{
    volatile static int i;

    public static class myT extends Thread
    {
        public void run ()
        {
            int j = 0;
            while(j<1000000){
                i++;
                j++;
            }
        }
    }

    public static void main (String[] argv)
    throws InterruptedException{
            i = 0;

            Thread my1 = new myT();
            Thread my2 = new myT();
            my1.start();
            my2.start();

            my1.join();
            my2.join();

            System.out.println("i = "+i);
    }
}

Since volatile builds happens-before relationship, the final value of i should be strictly 2000000. However, the actual result is nothing different from being without volatile for variable i. Can anyone explanation why it doesn't work here? Since i is declared volatile, it should be protected from memory inconsistency.

Answer 1

Can anyone explanation why it doesn't work here? Since i is declared volatile, it should be protected from memory inconsistency.

It is protected but unfortunately i++ is not an atomic operation. It is actually read/increment/store. So volatile is not going to save you from the race conditions between threads. You might get the following order of operations from your program:

1. thread #1 reads i, gets 10
2. right afterwards, thread #2 reads i, gets 10
3. thread #1 increments i to 11
4. thread #2 increments i to 11
5. thread #1 stores 11 to i
6. thread #2 stores 11 to i

As you can see, even though 2 increments have happened and the value has been properly synchronized between threads, the race condition means the value only went up by 1. See this nice looking explanation. Here's another good answer: Is a volatile int in Java thread-safe?

What you should be using are AtomicInteger which allows you to safely increment from multiple threads.

static final AtomicInteger i = new AtomicInteger(0);
...
        for (int j = 0; j<1000000; j++) {
            i.incrementAndGet();
        }