1

I have a Counter class that stores value as AtomicInteger. That class should be thread safe. I have method boolean consume(int number) that should decrement counter and return true if counter >= number, and should not change counter and return false if counter < number

class Counter {
   AtomicInteger counter = new AtomicInteger(initialValue);

   boolean consume(int number) {
     counter.accumulateAndGet(number, (prev, next) -> {
            if (number <= prev) {
                return prev - number;
            } else {
                // not modify the previous number;
                return prev;
            }
        });
       return ???
   }
}

And I don't know if the function applied or not. I found the following solution

boolean consume(int number) {
    AtomicBoolean result = new AtomicBoolean(false);
    counter.accumulateAndGet(number, (prev, next) -> {
            if (number <= prev) {
                result.set(true);
                return prev - number;
                // function applied
            } else {
                result.set(false);
                // not modify the previous number;
                return prev;
            }
    });
    return result.get();
}

but javadoc of accumulateAndGet sais:

The function should be side-effect-free, since it may be re-applied when attempted updates fail due to contention among threads.

So, my solution has side effects. Is it safe to use? If not, how can I get the same result?

a3dsfcv
  • 1,146
  • 2
  • 20
  • 35
  • 1
    I'd say yes, since the side effect is idempotent: if the first update fails and a new attempt is done, the result will be updated correctly. If you incremented an external value in the binary operator, that would be a different matter. – JB Nizet Dec 01 '19 at 14:22
  • I would strongly suggest that it would be clearer if you didn't use this method. Write out the standard `compareAndSet` loop only as `for (;;)` containing two exits. Even a standard loop retaining enough local information to determine which way it went, if you are desperate for a Single Entry Single Exit (SESE) style. – Tom Hawtin - tackline Dec 01 '19 at 15:15

1 Answers1

0

From the description, it sounds as if you want something like:

class Counter {
    private final int initialValue = 42; // make compile

    /** Non-negative count. */
    private final AtomicInteger counter = new AtomicInteger(initialValue);

    public boolean consume(int number) {
        for (;;) {
           int old = counter.get();
           int next = old-number;

           if (next >= 0) {
               if (counter.compareAndSet(old, next)) {
                   return true;
               };
           } else {
               return false;
           }
        }
    }
}

since it may be re-applied when attempted updates fail due to contention among threads.

The code in the question is fine with regarding to being executed one or more times, but it doesn't help to use convenience methods if they're not convenient.

Tom Hawtin - tackline
  • 145,806
  • 30
  • 211
  • 305