How not to mess up with AtomicReference and autoboxing

One day I found myself writing something suspiciously similar to AtomicInteger#incrementAndGet. Here’s a simplified example of what it looked like:

class SlightlyAtomicInteger {
    private final AtomicReference<Integer> reference = new AtomicReference<>(0);

    public int incrementAndGet() {
        while (true) {
            Integer oldValue = reference.get();
            Integer newValue = oldValue + 1;
            if (reference.compareAndSet(oldValue, newValue)) {
                return newValue;
            }
        }
    }
}

IDE inspection told me: “Psst, you can change newValue declaration to int”.
I thought: “Why not? Oh, and I’ll change oldValue to int too”.
As you may have guessed from the title, I shouldn’t have done that.

I ran a test which increments my SlightlyAtomicInteger many times, and it hanged. The test was multithreaded and had an ExecutorService, a CountDownLatch and other things which kind of distracted me, so I wasted some time trying to understand where exactly I messed up. Then I added a debug print statement for the newValue, ran the test again and saw the program’s last words:

...
126
127
128

Smells like an Integer cache! I changed oldValue declaration back to Integer and the problem was gone. Let’s take a look at the javadoc for AtomicReference#compareAndSet to see why it was happening:

Atomically sets the value to the given updated value if the current value == the expected value.

compareAndSwap works with objects and checks only reference equality. Autoboxing is done via Integer#valueOf, which caches values from -128 to 127 and returns the same references for them. But for other values, repeated boxing will return two different instances:

Iteration current value expected current value new value
N-1 126 from cache 126 from cache 127 from cache
N 127 from cache 127 from cache some instance of 128
N+1 some instance of 128 another instance of 128 some instance of 129

Starting with 128, references to “current” and “expected current” values stop matching at all. In other words, If I initialized my class with any number >= 128 instead of 0, then it would hang immediately on the first iteration.

Usually people advise to use primitives by default, unless you need a nullable value or something to put in a collection. This rule can be expanded to any class with generics, not just collections, as it turned out with AtomicReference.