From 62fb85785710f9249e943eb8f248facf28eda6f7 Mon Sep 17 00:00:00 2001 From: Nick Khyl Date: Wed, 15 Jan 2025 14:22:14 -0600 Subject: [PATCH] ipn/ipnserver: fix TestConcurrentOSUserSwitchingOnWindows I made a last-minute change in #14626 to split a single loop that created 1_000 concurrent connections into an inner and outer loop that create 100 concurrent connections 10 times. This introduced a race because the last user's connection may still be active (from the server's perspective) when a new outer iteration begins. Since every new client gets a unique ClientID, but we reuse usernames and UIDs, the server may let a user in (as the UID matches, which is fine), but the test might then fail due to a ClientID mismatch: server_test.go:232: CurrentUser(Initial): got &{S-1-5-21-1-0-0-1001 User-4 Client-2 false false}; want &{S-1-5-21-1-0-0-1001 User-4 Client-114 false false} In this PR, we update (*testIPNServer).blockWhileInUse to check whether the server is currently busy and wait until it frees up. We then call blockWhileInUse at the end of each outer iteration so that the server is always in a known idle state at the beginning of the inner loop. We also check that the current user is not set when the server is idle. Updates tailscale/corp#25804 Updates #14655 (found when working on it) Signed-off-by: Nick Khyl --- ipn/ipnserver/server_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ipn/ipnserver/server_test.go b/ipn/ipnserver/server_test.go index 97a616db8..e77901c35 100644 --- a/ipn/ipnserver/server_test.go +++ b/ipn/ipnserver/server_test.go @@ -251,6 +251,12 @@ func TestConcurrentOSUserSwitchingOnWindows(t *testing.T) { }() } wg.Wait() + + if err := server.blockWhileInUse(ctx); err != nil { + t.Fatalf("blockWhileInUse: %v", err) + } + + server.checkCurrentUser(nil) } } @@ -346,7 +352,14 @@ func (s *testIPNServer) makeTestUser(name string, clientID string) *ipnauth.Test func (s *testIPNServer) blockWhileInUse(ctx context.Context) error { ready, cleanup := s.zeroReqWaiter.add(&s.mu, ctx) - <-ready + + s.mu.Lock() + busy := len(s.activeReqs) != 0 + s.mu.Unlock() + + if busy { + <-ready + } cleanup() return ctx.Err() }