Does the following lock-free code exhibit a race-condition?

Question

Within the Kubernetes Go repo on Github.com,

There is a lock-free implementation of a HighWaterMark data-structure. This code relies on atomic operations to achieve thread-safe code that is free of data races.

// HighWaterMark is a thread-safe object for tracking the maximum value seen
// for some quantity.
type HighWaterMark int64

// Update returns true if and only if 'current' is the highest value ever seen.
func (hwm *HighWaterMark) Update(current int64) bool {
    for {
        old := atomic.LoadInt64((*int64)(hwm))
        if current <= old {
            return false
        }
        if atomic.CompareAndSwapInt64((*int64)(hwm), old, current) {
            return true
        }
    }
}

This code relies on the atomic.LoadInt64 and atomic.CompareAndSwapInt64 functions within the standard library to achieve data race free code...which I believe it does but I believe there is another problem of a race condition.

If two competing threads (goroutines) are executing such code there exists an edge case where after the atomic.LoadInt64 occurs in the first thread, the second thread could have swapped out for a higher value. But after the first thread thinks the current int64 is actually larger than the old int64 a swap will happen. This swap would then effectively lower the stored value due to observing a stale old value.

Answer 1

If another thread got in the middle, the CompareAndSwap would fail and the loop would start over.

Think of CompareAndSwap as

if actual == expected { 
   actual = newval
}

but done atomically

So this code says:

old = hwm // but done in a thread safe atomic read way
if old < current {
   set hwm to current if hwm == old // atomically compare and then set value
}

when CAS (CompareAndSwap) fails, it returns false, causing the loop to start over until either:

a) "current" is not bigger than hwm

or

b) It successfully performs Load and then CompareAndSwap without another thread in the middle