util/winutil/gp: deflake TestGroupPolicyReadLockClose

The test goroutine read lockCnt immediately after Lock returned, racing
with Close: close(lk.closing) wakes lockSlow's select, whose deferred
Add(-2) on lockCnt can run before Close's CAS clears the LSB. When that
happens, lockCnt is briefly 1 (3 - 2) instead of 0 (1 + 2 - 2 - 1),
producing "lockCnt: got 1; want 0".

Move the lockCnt assertion into the main test goroutine, after both
Close has returned and the Lock goroutine has finished, so both updates
have settled before we read.

Fixes #19647

Change-Id: Ia67036ff73a1beb528cbd621460db9048f3066ad
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This commit is contained in:
Brad Fitzpatrick
2026-05-05 20:29:22 +00:00
committed by Brad Fitzpatrick
parent 872d79089e
commit f844c8bc32

View File

@@ -106,23 +106,13 @@ func TestGroupPolicyReadLockClose(t *testing.T) {
doWithCustomEnterLeaveFuncs(t, func(gpLock *PolicyLock) {
done := make(chan struct{})
var lockErr error
go func() {
defer close(done)
err := gpLock.Lock()
if err == nil {
lockErr = gpLock.Lock()
if lockErr == nil {
defer gpLock.Unlock()
}
// We closed gpLock before the enter function returned.
// (*PolicyLock).Lock is expected to fail.
if err == nil || !errors.Is(err, ErrInvalidLockState) {
t.Errorf("(*PolicyLock).Lock: got %v; want %v", err, ErrInvalidLockState)
}
// gpLock must not be held as Lock() failed.
if lockCnt := gpLock.lockCnt.Load(); lockCnt != 0 {
t.Errorf("lockCnt: got %v; want 0", lockCnt)
}
}()
<-init
@@ -130,7 +120,22 @@ func TestGroupPolicyReadLockClose(t *testing.T) {
if err := gpLock.Close(); err != nil {
t.Fatalf("(*PolicyLock).Close failed: %v", err)
}
// Wait for Lock to fully unwind before reading lockCnt.
// Otherwise we race with Close clearing the LSB:
// close(lk.closing) wakes lockSlow's select, whose defer
// runs Add(-2) on lockCnt before Close completes its CAS,
// briefly leaving lockCnt at 1 instead of 0.
<-done
// We closed gpLock before the enter function returned.
// (*PolicyLock).Lock is expected to fail.
if lockErr == nil || !errors.Is(lockErr, ErrInvalidLockState) {
t.Errorf("(*PolicyLock).Lock: got %v; want %v", lockErr, ErrInvalidLockState)
}
// gpLock must not be held as Lock() failed.
if lockCnt := gpLock.lockCnt.Load(); lockCnt != 0 {
t.Errorf("lockCnt: got %v; want 0", lockCnt)
}
}, enter, leave)
}