From 373c60efe313daba7a2b9aa8cf622efab0c8f19c Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Tue, 25 Aug 2026 09:43:47 +0000 Subject: [PATCH] all: use sync.WaitGroup.Go Drops the Add/Done bookkeeping, which in batcher.go was spread across three functions. --- hscontrol/mapper/batcher.go | 12 +---- hscontrol/mapper/batcher_concurrency_test.go | 11 ++-- hscontrol/servertest/connect_race_test.go | 14 ++--- hscontrol/servertest/race_test.go | 8 +-- hscontrol/state/node_store_hostname_test.go | 10 ++-- hscontrol/state/node_store_test.go | 54 ++++++-------------- hscontrol/state/ping_test.go | 13 ++--- hscontrol/state/ssh_check_test.go | 14 ++--- integration/helpers.go | 16 ++---- 9 files changed, 46 insertions(+), 106 deletions(-) diff --git a/hscontrol/mapper/batcher.go b/hscontrol/mapper/batcher.go index 078ebd8b5..e21d86465 100644 --- a/hscontrol/mapper/batcher.go +++ b/hscontrol/mapper/batcher.go @@ -403,9 +403,7 @@ func (b *Batcher) Start() { return } - b.wg.Add(1) - - go b.doWork() + b.wg.Go(b.doWork) } func (b *Batcher) Close() { @@ -439,12 +437,8 @@ func (b *Batcher) Close() { } func (b *Batcher) doWork() { - defer b.wg.Done() - for i := range b.workers { - b.wg.Add(1) - - go b.worker(i + 1) + b.wg.Go(func() { b.worker(i + 1) }) } // Create a cleanup ticker for removing truly disconnected nodes @@ -467,8 +461,6 @@ func (b *Batcher) doWork() { } func (b *Batcher) worker(workerID int) { - defer b.wg.Done() - wlog := log.With().Int(zf.WorkerID, workerID).Logger() for { diff --git a/hscontrol/mapper/batcher_concurrency_test.go b/hscontrol/mapper/batcher_concurrency_test.go index 1c0684098..cbf53a811 100644 --- a/hscontrol/mapper/batcher_concurrency_test.go +++ b/hscontrol/mapper/batcher_concurrency_test.go @@ -142,19 +142,16 @@ func runConcurrently(t *testing.T, n int, fn func(i int)) int { ) for i := range n { - wg.Add(1) - - go func(idx int) { - defer wg.Done() + wg.Go(func() { defer func() { if r := recover(); r != nil { panics.Add(1) - t.Logf("panic in goroutine %d: %v", idx, r) + t.Logf("panic in goroutine %d: %v", i, r) } }() - fn(idx) - }(i) + fn(i) + }) } wg.Wait() diff --git a/hscontrol/servertest/connect_race_test.go b/hscontrol/servertest/connect_race_test.go index b9110ad92..ce1f9e339 100644 --- a/hscontrol/servertest/connect_race_test.go +++ b/hscontrol/servertest/connect_race_test.go @@ -96,22 +96,16 @@ func TestConnectDisconnectRace(t *testing.T) { start := make(chan struct{}) - wg.Add(2) - - go func() { - defer wg.Done() - + wg.Go(func() { <-start _, _ = srv.State().Disconnect(r2ID, gen) - }() - go func() { - defer wg.Done() - + }) + wg.Go(func() { <-start _, _ = srv.State().Connect(r2ID) - }() + }) close(start) wg.Wait() diff --git a/hscontrol/servertest/race_test.go b/hscontrol/servertest/race_test.go index bf6e6f0e9..8c5ec778c 100644 --- a/hscontrol/servertest/race_test.go +++ b/hscontrol/servertest/race_test.go @@ -494,16 +494,12 @@ func TestRaceBatcherContention(t *testing.T) { srv.App.Change(c) }) - wg.Add(1) - var c2 *servertest.TestClient - go func() { - defer wg.Done() - + wg.Go(func() { c2 = servertest.NewClient(t, srv, "rtcon-node2", servertest.WithUser(user)) - }() + }) wg.Wait() diff --git a/hscontrol/state/node_store_hostname_test.go b/hscontrol/state/node_store_hostname_test.go index da14f0496..8ac007307 100644 --- a/hscontrol/state/node_store_hostname_test.go +++ b/hscontrol/state/node_store_hostname_test.go @@ -104,17 +104,13 @@ func TestConcurrentPutNodeSameGivenNameAllUnique(t *testing.T) { var wg sync.WaitGroup results := make(chan string, N) - for i := range N { - wg.Add(1) - - go func(id int) { - defer wg.Done() - + for id := range N { + wg.Go(func() { n := createTestNode(types.NodeID(id+1), 1, "alice", "laptop") //nolint:gosec // test ids view := store.PutNode(n) results <- view.GivenName() - }(i) + }) } wg.Wait() diff --git a/hscontrol/state/node_store_test.go b/hscontrol/state/node_store_test.go index 04d4290bb..9d605ca6e 100644 --- a/hscontrol/state/node_store_test.go +++ b/hscontrol/state/node_store_test.go @@ -901,17 +901,13 @@ func TestNodeStoreConcurrentPutNode(t *testing.T) { var wg sync.WaitGroup results := make(chan bool, concurrentOps) - for i := range concurrentOps { - wg.Add(1) - - go func(nodeID int) { - defer wg.Done() - + for nodeID := 1; nodeID <= concurrentOps; nodeID++ { + wg.Go(func() { node := createConcurrentTestNode(types.NodeID(nodeID), "concurrent-node") //nolint:gosec // safe conversion in test resultNode := store.PutNode(node) results <- resultNode.Valid() - }(i + 1) + }) } wg.Wait() @@ -940,17 +936,13 @@ func TestNodeStoreBatchingEfficiency(t *testing.T) { var wg sync.WaitGroup results := make(chan bool, ops) - for i := range ops { - wg.Add(1) - - go func(nodeID int) { - defer wg.Done() - + for nodeID := 1; nodeID <= ops; nodeID++ { + wg.Go(func() { node := createConcurrentTestNode(types.NodeID(nodeID), "batch-node") //nolint:gosec // test code with small integers resultNode := store.PutNode(node) results <- resultNode.Valid() - }(i + 1) + }) } wg.Wait() @@ -988,12 +980,8 @@ func TestNodeStoreRaceConditions(t *testing.T) { errors := make(chan error, numGoroutines*opsPerGoroutine) - for i := range numGoroutines { - wg.Add(1) - - go func(gid int) { - defer wg.Done() - + for gid := range numGoroutines { + wg.Go(func() { for j := range opsPerGoroutine { switch j % 3 { case 0: @@ -1017,7 +1005,7 @@ func TestNodeStoreRaceConditions(t *testing.T) { } } } - }(i) + }) } wg.Wait() @@ -1097,14 +1085,10 @@ func TestNodeStoreOperationTimeout(t *testing.T) { updateResults := make([]error, ops) // Launch all PutNode operations concurrently - for i := 1; i <= ops; i++ { - nodeID := types.NodeID(i) //nolint:gosec // test code with small integers - - wg.Add(1) - - go func(idx int, id types.NodeID) { - defer wg.Done() + for idx := 1; idx <= ops; idx++ { + id := types.NodeID(idx) //nolint:gosec // test code with small integers + wg.Go(func() { startPut := time.Now() fmt.Printf("[TestNodeStoreOperationTimeout] %s: PutNode(%d) starting\n", startPut.Format("15:04:05.000"), id) node := createConcurrentTestNode(id, "timeout-node") @@ -1115,7 +1099,7 @@ func TestNodeStoreOperationTimeout(t *testing.T) { if !resultNode.Valid() { putResults[idx-1] = fmt.Errorf("PutNode failed for node %d", id) //nolint:err113 } - }(i, nodeID) + }) } wg.Wait() @@ -1123,14 +1107,10 @@ func TestNodeStoreOperationTimeout(t *testing.T) { // Launch all UpdateNode operations concurrently wg = sync.WaitGroup{} - for i := 1; i <= ops; i++ { - nodeID := types.NodeID(i) //nolint:gosec // test code with small integers - - wg.Add(1) - - go func(idx int, id types.NodeID) { - defer wg.Done() + for idx := 1; idx <= ops; idx++ { + id := types.NodeID(idx) //nolint:gosec // test code with small integers + wg.Go(func() { startUpdate := time.Now() fmt.Printf("[TestNodeStoreOperationTimeout] %s: UpdateNode(%d) starting\n", startUpdate.Format("15:04:05.000"), id) resultNode, ok := store.UpdateNode(id, func(n *types.Node) { @@ -1142,7 +1122,7 @@ func TestNodeStoreOperationTimeout(t *testing.T) { if !ok || !resultNode.Valid() { updateResults[idx-1] = fmt.Errorf("UpdateNode failed for node %d", id) //nolint:err113 } - }(i, nodeID) + }) } done := make(chan struct{}) diff --git a/hscontrol/state/ping_test.go b/hscontrol/state/ping_test.go index a51a851c8..2aa9558f1 100644 --- a/hscontrol/state/ping_test.go +++ b/hscontrol/state/ping_test.go @@ -1,6 +1,7 @@ package state import ( + "slices" "sync" "testing" "time" @@ -79,14 +80,10 @@ func TestPingTracker_ConcurrentDifferentIDs(t *testing.T) { // Complete in reverse order concurrently. var wg sync.WaitGroup - for i := count - 1; i >= 0; i-- { - wg.Add(1) - - go func(idx int) { - defer wg.Done() - - assert.True(t, pt.complete(ids[idx])) - }(i) + for _, id := range slices.Backward(ids) { + wg.Go(func() { + assert.True(t, pt.complete(id)) + }) } // All channels should receive. diff --git a/hscontrol/state/ssh_check_test.go b/hscontrol/state/ssh_check_test.go index 04b9d6b18..269484a21 100644 --- a/hscontrol/state/ssh_check_test.go +++ b/hscontrol/state/ssh_check_test.go @@ -85,19 +85,13 @@ func TestSSHCheckAuthConcurrent(t *testing.T) { wg.Wait() // Clear concurrently with reads - wg.Add(2) - - go func() { - defer wg.Done() - + wg.Go(func() { s.ClearSSHCheckAuth() - }() - - go func() { - defer wg.Done() + }) + wg.Go(func() { s.GetLastSSHAuth(types.NodeID(1), types.NodeID(2)) - }() + }) wg.Wait() } diff --git a/integration/helpers.go b/integration/helpers.go index 908c7447b..d7350d753 100644 --- a/integration/helpers.go +++ b/integration/helpers.go @@ -735,17 +735,11 @@ func assertClientsState(t *testing.T, clients []TailscaleClient) { var wg sync.WaitGroup for _, client := range clients { - wg.Add(1) - - c := client // Avoid loop pointer - - go func() { - defer wg.Done() - - assertValidStatus(t, c) - assertValidNetcheck(t, c) - assertValidNetmap(t, c) - }() + wg.Go(func() { + assertValidStatus(t, client) + assertValidNetcheck(t, client) + assertValidNetmap(t, client) + }) } t.Logf("waiting for client state checks to finish")