Java volatile keyword not working as expected

Question

I am learning volatile variable. I know what volatile does, i wrote a sample program for Volatile variable but not working as expected.

Why is the final value of the "count" coming sometime less then 2000. I have used volatile hence the system should not cache "count" variable and the value should always be 2000.

When I used synchronized method it work fine but not in the case of volatile keyword.

public class Worker {

private volatile int count = 0;
private int limit = 10000;

public static void main(String[] args) {
    Worker worker = new Worker();
    worker.doWork();
}

public void doWork() {
    Thread thread1 = new Thread(new Runnable() {
        public void run() {
            for (int i = 0; i < limit; i++) {

                    count++;

            }
        }
    });
    thread1.start();
    Thread thread2 = new Thread(new Runnable() {
        public void run() {
            for (int i = 0; i < limit; i++) {

                    count++;

            }
        }
    });
    thread2.start();

    try {
        thread1.join();
        thread2.join();
    } catch (InterruptedException ignored) {}
    System.out.println("Count is: " + count);
}
}

Thank You in advance !

Answer 1

When you do count++, that's a read, an increment, and then a write. Two threads can each do their read, each do their increment, then each do their write, resulting in only a single increment. While your reads are atomic, your writes are atomic, and no values are cached, that isn't enough. You need more than that -- you need an atomic read-modify-write operation, and volatile doesn't provide that.

Answer 2

count++ is basically this:

// 1. read/load count
// 2. increment count
// 3. store count
count = count + 1;

individually the first and third operation are atomic. All 3 of them together are not atomic.

Answer 3

i++ is not atomic in Java. So two threads may concurrently read, both calculate +1 to be the same number, and both store the same result.

Compile this using javac inc.java:

public class inc {
    static int i = 0;
    public static void main(String[] args) {
        i++;
    }
}

Read the bytecode using javap -c inc. I've trimmed this down to just show the function main():

public class inc {
  static int i;

  public static void main(java.lang.String[]);
    Code:
       0: getstatic     #2                  // Field i:I
       3: iconst_1
       4: iadd
       5: putstatic     #2                  // Field i:I
       8: return
}

We see that increment (of a static int) is implemented using: getstatic, iconst_1, iadd and putstatic.

Since this is done with four instructions and no locks, there can be no expectation of atomicity. Also worth noting that even if this were done with 1 instruction, we may be out of luck (quote from user "Hot Licks" comment in this thread):

Even on hardware that implements an "increment storage location" instruction, there's no guarantee that that's thread-safe. Just because an operation can be represented as a single operator says nothing about it's thread-safety.

---

If you actually wanted to solve this, you could use AtomicInteger, which has an atomicity guarantee:

final AtomicInteger myCoolInt = new AtomicInteger(0);
myCoolInt.incrementAndGet(1);