add more race condition prevention tests

Author: robbyt

runnables/httpcluster/runner_test.go

@@ -497,14 +497,12 @@ func TestRunnerConcurrency(t *testing.T) {
 		// Send multiple configs concurrently
 		const numConfigs = 10
 		var wg sync.WaitGroup
-		wg.Add(numConfigs)
 
 		testCtx, testCancel := context.WithCancel(t.Context())
 		defer testCancel()
 
-		for i := 0; i < numConfigs; i++ {
-			go func(i int) {
-				defer wg.Done()
+		for range numConfigs {
+			wg.Go(func() {
 				// Each concurrent update uses a different port
 				addr := networking.GetRandomListeningPort(t)
 				configs := map[string]*httpserver.Config{
@@ -519,7 +517,7 @@ func TestRunnerConcurrency(t *testing.T) {
 				case <-ctx.Done():
 					// Timeout is ok for buffered channel
 				}
-			}(i)
+			})
 		}
 
 		wg.Wait()
@@ -553,11 +551,9 @@ func TestRunnerConcurrency(t *testing.T) {
 		// Read state concurrently
 		const numReaders = 50
 		var wg sync.WaitGroup
-		wg.Add(numReaders)
 
-		for i := 0; i < numReaders; i++ {
-			go func() {
-				defer wg.Done()
+		for range numReaders {
+			wg.Go(func() {
 				state := runner.GetState()
 				assert.NotEmpty(t, state)
 
@@ -566,7 +562,7 @@ func TestRunnerConcurrency(t *testing.T) {
 
 				str := runner.String()
 				assert.Contains(t, str, "HTTPCluster")
-			}()
+			})
 		}
 
 		wg.Wait()
@@ -593,13 +589,11 @@ func TestRunnerConcurrency(t *testing.T) {
 		// Call Stop concurrently
 		const numStops = 10
 		var wg sync.WaitGroup
-		wg.Add(numStops)
 
-		for i := 0; i < numStops; i++ {
-			go func() {
-				defer wg.Done()
+		for range numStops {
+			wg.Go(func() {
 				runner.Stop()
-			}()
+			})
 		}
 
 		wg.Wait()

runnables/httpserver/runner_race_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -159,3 +160,50 @@ func TestRunnerRaceConditions(t *testing.T) {
 
 	<-errChan
 }
+
+// TestGetConfig_ConcurrentAccess attempts to prove a race condition in getConfig()
+// when multiple goroutines call it concurrently with nil config.
+// Since NewRunner eagerly loads config during construction, all concurrent
+// getConfig() calls find non-nil config and never hit the callback path.
+func TestGetConfig_ConcurrentAccess(t *testing.T) {
+	t.Parallel()
+
+	var callbackCount atomic.Int64
+
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}
+	route, err := NewRouteFromHandlerFunc("test", "/test", handler)
+	require.NoError(t, err)
+
+	port := fmt.Sprintf(":%d", networking.GetRandomPort(t))
+
+	cfgCallback := func() (*Config, error) {
+		callbackCount.Add(1)
+		return NewConfig(port, Routes{*route}, WithDrainTimeout(1*time.Second))
+	}
+
+	runner, err := NewRunner(WithConfigCallback(cfgCallback))
+	require.NoError(t, err)
+
+	// NewRunner calls getConfig() once during construction
+	assert.Equal(t, int64(1), callbackCount.Load())
+
+	const goroutines = 10
+	configs := make([]*Config, goroutines)
+	var wg sync.WaitGroup
+	for i := range goroutines {
+		wg.Go(func() {
+			configs[i] = runner.getConfig()
+		})
+	}
+	wg.Wait()
+
+	// Callback should still only have been called once — config was already loaded by NewRunner
+	assert.Equal(t, int64(1), callbackCount.Load())
+
+	// All goroutines got the same config
+	for i := range configs {
+		assert.Same(t, configs[0], configs[i])
+	}
+}