Search code examples
javaatomicvolatile

replace `getIntVolatile(Object var1, long var2)` with `getInt(Object var1, long var2)` in the implementation of incrementAndGet() in java


According the jdk ,the implementation of the method incrementAndGet() of AtomicInteger class is as follows:

public final int incrementAndGet() {
    return unsafe.getAndAddInt(this, valueOffset, 1) + 1;
}
public final int getAndAddInt(Object var1, long var2, int var4) {
    int var5;
    do {
        var5 = this.getIntVolatile(var1, var2);
    } while(!this.compareAndSwapInt(var1, var2, var5, var5 + var4));

    return var5;
}

What happens if the getIntVolatile(Object var1, long var2) method is replaced by getInt(Object var1, long var2) method?

What I think is that the getIntVolatile(Object var1, long var2) method is unnecessary, as the operated variable is a volatile variable,the getInt(Object var1, long var2) is able to read the latest value.


Solution

  • Look at what incrementAndGet does:

    return unsafe.getAndAddInt(this, valueOffset, 1) + 1;
    

    unsafe has no idea that at the valueOffset there is a volatile field and it must treat it as such. So, this (I am not using var2 and the like, but the real source code):

    v = getIntVolatile(o, offset);
    

    surely looks like protects against some possible re-orderings. Let's say you replace that getIntVolatile with getInt:

     do {
          v = getInt(o, offset);
     } while (!weakCompareAndSetInt(o, offset, v, v + delta));
    

    Since there are no volatile semantics here, the compiler can move the read out of the loop:

     int v = getInt(o, offset);
     do {
     } while (!weakCompareAndSetInt(o, offset, v, v + delta));
    

    If it does that, you can end-up in an infinite loop, pretty trivially.