From 261de6160bffcf9916c6f283e8b855b9a64379fb Mon Sep 17 00:00:00 2001 From: DarrylWong Date: Thu, 13 Feb 2025 11:24:22 -0500 Subject: [PATCH] roachtestutil: httpclient grabs lock earlier Before, the defaultHTTPClient would grab the lock only when adding cookies. However, we also want to protect the check of a sessionID from concurrent access as well. Previously, it was possible that concurrent calls would all see the sessionID as not set and all attempt to authenticate with the cluster. --- pkg/cmd/roachtest/roachtestutil/httpclient.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/roachtest/roachtestutil/httpclient.go b/pkg/cmd/roachtest/roachtestutil/httpclient.go index 90617951344d..8ff241ef1f85 100644 --- a/pkg/cmd/roachtest/roachtestutil/httpclient.go +++ b/pkg/cmd/roachtest/roachtestutil/httpclient.go @@ -134,6 +134,9 @@ func (r *RoachtestHTTPClient) addCookies(ctx context.Context, cookieUrl string) if !r.cluster.IsSecure() { return nil } + + r.mu.Lock() + defer r.mu.Unlock() // If we haven't extracted the sessionID yet, do so. if r.sessionID == "" { id, err := getSessionID(ctx, r.cluster, r.l, r.cluster.All(), r.virtualClusterName) @@ -169,8 +172,6 @@ func (r *RoachtestHTTPClient) addCookies(ctx context.Context, cookieUrl string) // SetCookies is a helper that checks if a client.CookieJar exists and creates // one if it doesn't. It then sets the provided cookies through CookieJar.SetCookies. func (r *RoachtestHTTPClient) SetCookies(u *url.URL, cookies []*http.Cookie) error { - r.mu.Lock() - defer r.mu.Unlock() if r.client.Jar == nil { jar, err := cookiejar.New(nil) if err != nil {