Java atomic classes in compound operations

Will the following code cause race condition issue if several threads invoke the "incrementCount" method?

public class sample {

       private AtomicInteger counter = new AtomicInteger(0);

       public int getCurren开发者_运维知识库tCount {
          int current = counter.getAndIncrement();
          if (counter.compareAndSet(8, 0)) current = 0;
          return current;
      }
}

If it causes race condition, what are the possible solution other than using synchronized keyword?

You probably don't want to let the counter exceed 8 and this won't work. There are race conditions.

It looks like you want a mod 8 counter. The easiest way is to leave the AtomicInteger alone and use something like

int current = counter.getAndIncrement() & 7;

(which is fixed and optimized version of % 8). For computations mod 8 or any other power of two it works perfectly, for other number you'd need % N and get problems with int overflowing to negative numbers.

The direct solution goes as follows

public int getCurrentCount {
    while (true) {
        int current = counter.get();
        int next = (current+1) % 8;
        if (counter.compareAndSet(current, next))) return next;
    }
}

This is about how getAndIncrement() itself works, just slightly modified.

Yes, it probably does not do what you want (there is a kind of race condition).

1. One thread may call getAndIncrement() and receive a 8
2. A second thread may call getAndIncrement() and receive a 9
3. The first thread tries compareAndSet but the value is not 8
4. The second thread tries compareAndSet but the value is not 8

If there's no risk of overflowing, you could do something like

return counter.getAndIncrement() % 8;

Relying on that something does not overflow seems like a poor idea to me though, and I would probably do roughly what you do, but let the method be synchronized.

• Related question: Modular increment with Java's Atomic classes

What are you trying to achieve? Even if you use the fixes proposed by ajoobe or maartinus you can end up with different threads getting the same answer - consider 20 threads running simultaneously. I don't see any interesting significance of this "counter" as you present it here - you may as well just pick a random number between 0 and 8.

Based on the code for getAndIncrement()

public int getCurrentCount() {
  for(;;) {
    int courrent = counter.get();
    int next = current + 1;
    if (next >= 8) next = 0;
    if (counter.compareAndSet(current, next))
      return current;
  }
}

However a simpler implementation in your case is to do

public int getCurrentCount() {
  return counter.getAndIncrement() & 0x7;
}

I assume that the what you want is to have a counter form 0 to 7.

If that is the case then a race condition can possibly happen and the value of counter can become 9.

Unless you are ok to use % soln. as said by others, you micht have to use synchronized.