From f844c8bc32f237bf21f83ce89923cafc6c830a00 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Tue, 5 May 2026 20:29:22 +0000
Subject: [PATCH] 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>
---
 util/winutil/gp/gp_windows_test.go | 31 +++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/util/winutil/gp/gp_windows_test.go b/util/winutil/gp/gp_windows_test.go
index dfad02930..1ce75871e 100644
--- a/util/winutil/gp/gp_windows_test.go
+++ b/util/winutil/gp/gp_windows_test.go
@@ -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)
 }