Reputation: 384
I ran across a doubt if below is thread safe,
// is this thread safe, final int MAX_COUNT = 3 ?
if (retryCount.get() < MAX_COUNT) {
// some other code
retryCount.getAndIncrement();
} else {
// reset count & some other code
retryCount.set(0);
}
Is the above conditional check thread safe ?
Upvotes: 2
Views: 1034
Reputation: 131526
No it doesn't.
Suppose two threads T1
and T2
and retryCount
contains actually the 2
value.
Suppose T1
executes if(retryCount.get() < MAX_COUNT){
(evaluated to true) but doesn't reach retryCount.getAndIncrement();
.
T1
is paused. T2
is resumed.
T2
executes if(retryCount.get() < MAX_COUNT){
that is still evaluated to true.
So you are sure that retryCount
will be valued to 4
.
You need explicit synchronization and in this case the AtomicInteger
may not be required :
synchronized(lock){
if(retryCount.get() < MAX_COUNT){
// some other code
retryCount.getAndIncrement();
}else{
// reset count & some other code
retryCount.set(0);
}
}
Upvotes: 4
Reputation: 8354
No it's not thread safe because the code is performing what's called a check-then-act
operation.
AtomicInteger
is thread safe in itself meaning it's individual methods are atomic but performing compound operations are not atomic. so the above code needs synchronization
here's some great notes from Java Concurrency In Practice
Upvotes: 1