AtomicInteger and volatile
I'd say no, it's not thread-safe, if you define thread-safe as having the same result under single threaded mode and multithreaded mode. In single threaded mode, the count will never go greater than 10, but in multithreaded mode it can.
The issue is that get
and incrementAndGet
is atomic but an if
is not. Keep in mind that a non-atomic operation can be paused at any time. For example:
count = 9
currently.- Thread A runs
if(count.get() <10)
and getstrue
and stopped there. - Thread B runs
if(count.get() <10)
and getstrue
too so it runscount.incrementAndGet()
and finishes. Nowcount = 10
. - Thread A resumes and runs
count.incrementAndGet()
, nowcount = 11
which will never happen in single threaded mode.
If you want to make it thread-safe without using synchronized
which is slower, try this implementation instead:
class A{
final AtomicInteger count;
void someMethod(){
// do something
if(count.getAndIncrement() <10){
// safe now
} else count.getAndDecrement(); // rollback so this thread did nothing to count
}
To maintain the original semantics, and support multiple threads, you could do something like:
public class A {
private AtomicInteger count = new AtomicInteger(0);
public void someMethod() {
int i = count.get();
while (i < 10 && !count.compareAndSet(i, i + 1)) {
i = count.get();
}
}
}
This avoids any thread ever seeing count reach 10.
I believe that Atomic*
actually gives both atomicity and volatility. So when you call (say) AtomicInteger.get()
, you're guaranteed to get the latest value. This is documented in the java.util.concurrent.atomic
package documentation:
The memory effects for accesses and updates of atomics generally follow the rules for volatiles, as stated in section 17.4 of The Java™ Language Specification.
- get has the memory effects of reading a volatile variable.
- set has the memory effects of writing (assigning) a volatile variable.
- lazySet has the memory effects of writing (assigning) a volatile variable except that it permits reorderings with subsequent (but not previous) memory actions that do not themselves impose reordering constraints with ordinary non-volatile writes. Among other usage contexts, > - lazySet may apply when nulling out, for the sake of garbage collection, a reference that is never accessed again.
- weakCompareAndSet atomically reads and conditionally writes a variable but does not create any happens-before orderings, so provides no guarantees with respect to previous or subsequent reads and writes of any variables other than the target of the weakCompareAndSet.
- compareAndSet and all other read-and-update operations such as getAndIncrement have the memory effects of both reading and writing volatile variables.
Now if you have
volatile AtomicInteger count;
the volatile
part means that each thread will use the latest AtomicInteger
reference, and the fact that it's an AtomicInteger
means that you'll also see the latest value for that object.
It's not common (IME) to need this - because normally you wouldn't reassign count
to refer to a different object. Instead, you'd have:
private final AtomicInteger count = new AtomicInteger();
At that point, the fact that it's a final
variable means that all threads will be dealing with the same object - and the fact that it's an Atomic*
object means they'll see the latest value within that object.