From 3ef137a8d2e3c59ffb0a95ed8924236aad27ef55 Mon Sep 17 00:00:00 2001 From: Brad Davidson Date: Thu, 6 Jun 2024 23:47:27 +0000 Subject: [PATCH] Fix race condition panic in loadbalancer.nextServer Signed-off-by: Brad Davidson --- pkg/agent/loadbalancer/servers.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/pkg/agent/loadbalancer/servers.go b/pkg/agent/loadbalancer/servers.go index 7dc80e4932..3564a6a4ee 100644 --- a/pkg/agent/loadbalancer/servers.go +++ b/pkg/agent/loadbalancer/servers.go @@ -115,23 +115,33 @@ func (lb *LoadBalancer) nextServer(failedServer string) (string, error) { lb.mutex.RLock() defer lb.mutex.RUnlock() + // note: these fields are not protected by the mutex, so we clamp the index value and update + // the index/current address using local variables, to avoid time-of-check vs time-of-use + // race conditions caused by goroutine A incrementing it in between the time goroutine B + // validates its value, and uses it as a list index. + currentServerAddress := lb.currentServerAddress + nextServerIndex := lb.nextServerIndex + if len(lb.randomServers) == 0 { return "", errors.New("No servers in load balancer proxy list") } if len(lb.randomServers) == 1 { - return lb.currentServerAddress, nil + return currentServerAddress, nil } - if failedServer != lb.currentServerAddress { - return lb.currentServerAddress, nil + if failedServer != currentServerAddress { + return currentServerAddress, nil } - if lb.nextServerIndex >= len(lb.randomServers) { - lb.nextServerIndex = 0 + if nextServerIndex >= len(lb.randomServers) { + nextServerIndex = 0 } - lb.currentServerAddress = lb.randomServers[lb.nextServerIndex] - lb.nextServerIndex++ + currentServerAddress = lb.randomServers[nextServerIndex] + nextServerIndex++ - return lb.currentServerAddress, nil + lb.currentServerAddress = currentServerAddress + lb.nextServerIndex = nextServerIndex + + return currentServerAddress, nil } // dialContext dials a new connection using the environment's proxy settings, and adds its wrapped connection to the map