From 84aa7ff3bbcabd1bbc272c99de0c898b799cb144 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Fri, 30 May 2025 08:06:16 -1000 Subject: [PATCH] syncs: fix AtomicValue.CompareAndSwap (#16137) Fix CompareAndSwap in the edge-case where the underlying sync.AtomicValue is uninitialized (i.e., Store was never called) and the oldV is the zero value, then perform CompareAndSwap with any(nil). Also, document that T must be comparable. This is a pre-existing restriction. Fixes #16135 Signed-off-by: Joe Tsai --- syncs/syncs.go | 10 ++++++++-- syncs/syncs_test.go | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/syncs/syncs.go b/syncs/syncs.go index 337fca755..cf0be919b 100644 --- a/syncs/syncs.go +++ b/syncs/syncs.go @@ -67,12 +67,18 @@ func (v *AtomicValue[T]) Swap(x T) (old T) { if oldV != nil { return oldV.(wrappedValue[T]).v } - return old + return old // zero value of T } // CompareAndSwap executes the compare-and-swap operation for the Value. +// It panics if T is not comparable. func (v *AtomicValue[T]) CompareAndSwap(oldV, newV T) (swapped bool) { - return v.v.CompareAndSwap(wrappedValue[T]{oldV}, wrappedValue[T]{newV}) + var zero T + return v.v.CompareAndSwap(wrappedValue[T]{oldV}, wrappedValue[T]{newV}) || + // In the edge-case where [atomic.Value.Store] is uninitialized + // and trying to compare with the zero value of T, + // then compare-and-swap with the nil any value. + (any(oldV) == any(zero) && v.v.CompareAndSwap(any(nil), wrappedValue[T]{newV})) } // MutexValue is a value protected by a mutex. diff --git a/syncs/syncs_test.go b/syncs/syncs_test.go index 901d42948..2439b6068 100644 --- a/syncs/syncs_test.go +++ b/syncs/syncs_test.go @@ -64,6 +64,23 @@ func TestAtomicValue(t *testing.T) { t.Fatalf("LoadOk = (%v, %v), want (nil, true)", got, gotOk) } } + + { + c1, c2, c3 := make(chan struct{}), make(chan struct{}), make(chan struct{}) + var v AtomicValue[chan struct{}] + if v.CompareAndSwap(c1, c2) != false { + t.Fatalf("CompareAndSwap = true, want false") + } + if v.CompareAndSwap(nil, c1) != true { + t.Fatalf("CompareAndSwap = false, want true") + } + if v.CompareAndSwap(c2, c3) != false { + t.Fatalf("CompareAndSwap = true, want false") + } + if v.CompareAndSwap(c1, c2) != true { + t.Fatalf("CompareAndSwap = false, want true") + } + } } func TestMutexValue(t *testing.T) {