Skip to content

Commit

Permalink
roachtestutil: httpclient grabs lock earlier
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DarrylWong committed Feb 13, 2025
1 parent 7eb988a commit 261de61
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions pkg/cmd/roachtest/roachtestutil/httpclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 261de61

Please sign in to comment.