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.
