AtomicInteger for limited sequnce generation

Question

How can we use AtomicInteger for limited sequence generation say the sequence number has to be between 1 to 60. Once the sequece reaches 60 it has to start again from 1. I wrote this code though not quite sure wether this is thread safe or not?

public int getNextValue()
{
 int v;
 do
 {
   v = val.get();
   if ( v == 60)
   {
    val.set(1);
   }
 }
  while (!val.compareAndSet(v , v + 1));
   return v + 1;
  }

Answer 1

You could do

return val.getAndIncrement() % 60;

Unless you're concerned with exceeding the integer max-value (2147483647). If that is a concern, you could have a look at the getAndIncrement implementation:

public final int getAndIncrement() {
    for (;;) {
        int current = get();
        int next = current + 1;
        if (compareAndSet(current, next))
            return current;
    }
}

All you need to change is the int next... line to something like:

int next = (current + 1) % 60;

---

Oops. This loops through 0->59. You needed 1->60, so add one to the return-value to get the desired result.

Answer 2

No, it's not thread safe - you shouldn't call set inside a cycle:

int value, next;
do {
    value = val.get();
    next = (value == 60) ? 1 : (value + 1);
} while (!val.compareAndSet(value, next);
return next;

Answer 3

If you make the method synchronized then it will be threadsafe as long as the val is nowhere else accessed. The approach is however a bit cumbersome, I'd rewrite it as follows:

public synchronized int getNextValue() {
    val.compareAndSet(60, 0); // Set to 0 if current value is 60.
    return val.incrementAndGet();
}

This gives 1 until with 60 back inclusive. If you actually need 1 until with 59, then replace 60 by 59.

Answer 4

Any particular reason to use AtomicInteger here rather than just a simple synchronized method?

How about something simple like the following:

private int val=1;

public synchronized int getNextValue() {
 int v=val;
 val = (val==60) ? 1 : (val+1); 
 return v;
}

Answer 5

Quick answer, not thread safe. The test and set need to be atomic, unless you make the whole method synchronized. Note the val.get() and the test of v are not atomic. If the thread yields after v = val.get() you'll get two calls with the same sequence number.

Also, if the compareAndSet fails you never change the values, it'll be an infinite loop.

AtomicInteger has a getAndIncrement() call. That will get you a clean value to return.

The rolling is a bit trickier. One solution is to mod the return value. Something like so:

int v = val.getAndIncrement();
return (v % 60) + 1;

Since each thread has a local copy of v we can safely do some math on it and return the value. There is one sticking point if you get an overflow. Depending on how often you generate a sequence number this may or may not be an issue.