Vijay
Vijay

Reputation: 384

AtomicInteger conditional check thread safe

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

Answers (2)

davidxxx
davidxxx

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

Ramanlfc
Ramanlfc

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

Related Questions