Is each cycle of a for loop an atomic operation?-CodePudding

Is the incrementAndGet method of the following AtomicBigInteger implementation an atomic operation? I'm particularly wondering about the for (; ; ) part. Does the JVM somehow guarantee that each cycle in a for loop is executed atomicly?

public final class AtomicBigInteger {

    private final AtomicReference<BigInteger> valueHolder = new AtomicReference<>();

    public AtomicBigInteger(BigInteger bigInteger) {
        valueHolder.set(bigInteger);
    }

    public BigInteger incrementAndGet() {
        for (; ; ) {
            BigInteger current = valueHolder.get();
            BigInteger next = current.add(BigInteger.ONE);
            if (valueHolder.compareAndSet(current, next)) {
                return next;
            }
        }
    }
}

I got this code from here: Possible to safely increment BigInteger in a thread safe way, perhaps with AtomicReference, w/o locking? However this implementation was making its rounds and you can find it on many different places across the internet.

CodePudding user response：

No, it is not atomic, but if another thread has modified the AtomicReference then the compareAndSet call will fail and it will loop again, get the value, increment it, and try to set it again. Sooner or later (probably) it will succeed and update the BigInteger held by the AtomicReference to the next number.

CodePudding user response：

The method incrementAndGet in your class will not be atomic. Here's why.

The Atomic* classes use volatile value references. The memory offset to these values are also held inside the instances using which they are able to fetch-increment-compare-set in loop till the current thread is able to do all operations in one go (i.e., without another thread performing an increment in between).

This is possible for these Atomic*, as I see, due to the access the intrinsic "trusted" classes have to the Unsafe implementations. The Unsafe implementations have methods to compare-and-set atomically using native functions.

In cases like what you have mentioned, we will have to resort to either using synchronized blocks, its equivalent Lock based implementations or simply use the methods in AtomicReference. Like this:

public class AtomicBigInteger{
    private final AtomicReference<BigInteger> valueHolder = new AtomicReference<>();

    public AtomicBigInteger(BigInteger bigInteger) {
        valueHolder.set(bigInteger);
    }

    public BigInteger incrementAndGet() {
        return valueHolder.updateAndGet( bigInt -> bigInt.add( BigInteger.ONE ) );
    } 
}

However, since we are dealing with BigInteger, this implementation too will have to be reviewed because the number of iterations that AtomicReference.updateAndGet(..) may have to perform may be significant, because BigInteger.add( BigInteger ) involves a lot of steps, unlike an addition of two ints.