Question

I have the below code

class VolatileCount {
    volatile int count;
    Object lock = new Object();

    public void increment() {

        synchronized (lock) {
            count = count + 1;
        }
        System.out.print(" " + count);
    }

}

If I call increment() on same object from multiple threads I get the below output (might be different on your machine)

2 3 2 5 4 8 8 6 11 13 10 9 15 14 12 20 19

Looking at the repeating numbers I think happens-before seems to be broken, because considering first three number (2 3 2), if a thread sees 3 , increment has happened and since the variable is volatile, its value should be 3 or more but cannot be 2 in any thread.
However, the print line seems to have been reordered here, is it correct to reorder that line? What am I missing here? I run on JDK 7 (Eclipse)

Was it helpful?

Solution

Update

Because you seem to want an explanation for the specific case of "2 3 2"

  • X increments i (now 1)
  • Y increments i (now 2)
  • X reads i (X loads 2)
  • Y reads i (Y loads 2)
  • Y prints the value previously loaded (prints 2)
  • Z increments i, reads i, prints i (prints 3)
  • X prints the value previously loaded (prints 2)

The point is: System.out.print(" " + count) isn't atomic. The thread can be preempted after performing a volatile read and before printing the value.

If you want to prevent duplicate values from being printed, you have to perform the volatile read inside the lock:

public void increment() {
    int localCount;
    synchronized (lock) {
        count = count + 1;
        localCount = count; // volatile load
    }
    System.out.print(" " + localCount);
}

This wouldn't prevent values from being printed out of order though. In order to print them in order, without duplicates, you'd have to move the print into the lock as well.

Old Answer

The print statement is outside the lock. Consider the code running inside System.out.print(" " + count).

  • A thread X increments i
  • Thread X evaluates the print arguments and performs a volatile read on the count variable, and loads the value 2.
  • Thread X is preempted by Thread Y, which increments i
  • Thread Y loads i (which is now 3), calls the print method which runs to completion.
  • Thread Y is preempted, and Thread X now runs print to completion, which prints 2.

This would make the numbers show out of order, such as "3 2 4".

Some numbers might also be repeated when:

  • Thread X increments i (which is now 2)
  • Thread Y increments i(which is now 3)
  • Thread X prints i (which is 3)
  • Thread Y prints i (which is 3)

OTHER TIPS

consider 2 threads. First thread gets lock, increments count to 1 and releases the lock. Second thread gets lock, increments count to 2, releases lock, prints 2, then first thread gets the value of count (now 2) to print it. in this case you would get 2 2

specifically to your example:

  1. Thread A increments count to 1
  2. Thread B increments it to 2
  3. Thread A gets and prints value (2)
  4. Thread B gets value (2)
  5. Thread C inrements it to 3 and prints it
  6. Thread B prints value (still 2 since that is what it got)

there you have 2 3 2 etc...

The problem may lay on the System.out.print multithreading check this discussion and maybe try running it form the command line , since your IDE may buffering the output

If you put the print inside the lock , it should solve the problem

Your printout is outside the protected zone. That means you may be interrupted any time between the increment and the print occurring, including after count has been fetched but before it has been sent to System.out. If you move that inside the synchronized block, I believe you'll see the behavior you expect.

Volatile is not a replacement for synchronization. It just warns the compiler that the value may change at any time and therefore should be fetched each time it's referenced rather than being optimized out.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top