4

the source code of getAndIncrement is:

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

I don't understand why there is a loop. If some other threads have changed the value, then how can it be atomic?

let's say the value is 5, then I call getAndIncrement(), we expect it to be 6, but at the same time some other threads have changed the value to 6, then getAndIncrement() will make the value to 7, which is not expected.

Where am I wrong?

7
  • @yshavit the link you mentioned hasn't answered my question, can you check it? Thanks! Commented Mar 13, 2017 at 15:54
  • @yshavit so why does it retry? I don't understand this. Let's say its original value is 5, now I want to make it 6, but if some other threads have made it 6 , why should it retry to make it 7? Commented Mar 13, 2017 at 15:56
  • The method is atomic as it doesn't suffer from race conditions and is thread safe. if a thread sets it to 6 and you call getAndIncrement you would expect it to return 7. Commented Mar 13, 2017 at 15:56
  • 6
    @Lily Well, one of the threads needs to return 7, right? If you start with 5, and two threads increment it atomically, then one should return 6 while the other returns 7. Commented Mar 13, 2017 at 15:56
  • 4
    consider what this would do; public synchronized int getAndIncrement() { return value++; } how is this any different? Commented Mar 13, 2017 at 15:58

4 Answers 4

7

The loop will keep going until it manages to do the get(), the +1, and the compareAndSet without any other thread getting in a compareAndSet first. If another thread does get a compareAndSet in, then this thread's compareAndSet will fail, and the loop will retry.

The end result is that each call to getAndIncrement() will result in exactly one increment to the value. If the value is initially 5, and two threads call getAndIncrement(), then one will return 6 and the other will return 7.

Put another way: one of them will appear to happen fully after the other, which is what "atomic" means.

Sign up to request clarification or add additional context in comments.

4 Comments

so why does it retry? I don't understand this. Let's say its original value is 5, now I want to make it 6, but if some other threads have made it 6 , why should it retry to make it 7?
You started at 5, 2 things incremented it - something must be 7 eventually.
@Lily If you wanted that, it wouldn't be atomic, nor threadsafe.
Good stuff. Another important thing to point out is that the entire point of the loop is to avoid a synchronized block. This is trying to update a shared field without having to pay for the overhead of a lock.
0

As already answered,

each call to getAndIncrement() will result in exactly one increment to the value

Confusion seems to stem from your comment

Let's say its original value is 5, now I want to make it 6, but if some other threads have made it 6 , why should it retry to make it 7

Okey, so you want the system to behave one way, but the methods you are using are designed to do different. getAndIncrement is designed to ensure every invocation causes an increment, what you want is all invocations combined cause ONE increment. So clearly getAndIncrement should not be used here.

It is worth noting that the behavior you expect is rarely encountered in single-machine system, but frequently in distributed-system. If you are not doing distributed, then other people are right in finding fault in your approach.

Comments

0

The key to understanding this is to understand what compareAndSet() does:

/** * Atomically sets the value to the given updated value * if the current value {@code ==} the expected value. * * @param expect the expected value * @param update the new value * @return true if successful. False return indicates that * the actual value was not equal to the expected value. */ public final boolean compareAndSet(int expect, int update) { return unsafe.compareAndSwapInt(this, valueOffset, expect, update); } 

In Unsafe.java:

/** * Atomically update Java variable to <tt>x</tt> if it is currently * holding <tt>expected</tt>. * @return <tt>true</tt> if successful */ public final native boolean compareAndSwapInt(Object o, long offset, int expected, int x); 

So this method uses JVM internals to atomically:

  • check whether the value has the expected value
    • if not, do nothing and return false
    • if so, set to the new value and return true

The loop you've found exits when compareAndSet() returns true.

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

... is equivalent to:

boolean done = false; int current; while(!done) { current = get(); int next = current + 1; done = compareAndSet(current, next); } return current; 

... but slightly terser and cleaner.

Comments

0

@Lily, as @yshavit explains, the compareAndSet will only succeed if current is still valid and the counter was not updated by another thread. So it atomically updates the counter or it will return false. So it will continue iterating until it eventually succeeds. Current and next are recalculated on each iteration. So it will update the counter by exactly 1 or not at all.

This is a form of optimistic locking, which means that instead of having locks where other threads have to check whether they can proceed or have to wait, it does not lock at all and simply keeps trying opportunistically until it succeeds. The rationale is that this is cheaper than having synchronized blocks because typically the overhead for that is not needed and iterating and trying again is cheaper than having locks around code blocks.

Btw. in Oracle java 8, the implementation has changed and it now uses sun.misc.Unsafe internally, which probably calls some native logic to achieve the same goal.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.