Make ReqInfo concurrency safe (#15204)

Some read/writes of ReqInfo did not get appropriate locks, leading to races.

Make sure reading and writing holds appropriate locks.
This commit is contained in:
Klaus Post 2022-06-30 10:48:50 -07:00 committed by GitHub
parent 8856a2d77b
commit 9004d69c6f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 11 additions and 6 deletions

View file

@ -792,6 +792,8 @@ func likelyUnescapeGeneric(p string, escapeFn func(string) (string, error)) stri
func updateReqContext(ctx context.Context, objects ...ObjectV) context.Context {
req := logger.GetReqInfo(ctx)
if req != nil {
req.Lock()
defer req.Unlock()
req.Objects = make([]logger.ObjectVersion, 0, len(objects))
for _, ov := range objects {
req.Objects = append(req.Objects, logger.ObjectVersion{

View file

@ -149,7 +149,6 @@ func GetAuditEntry(ctx context.Context) *audit.Entry {
DeploymentID: xhttp.GlobalDeploymentID,
Time: time.Now().UTC(),
}
SetAuditEntry(ctx, r)
return r
}
return nil
@ -168,6 +167,8 @@ func AuditLog(ctx context.Context, w http.ResponseWriter, r *http.Request, reqCl
if reqInfo == nil {
return
}
reqInfo.RLock()
defer reqInfo.RUnlock()
entry = audit.ToEntry(w, r, reqClaims, xhttp.GlobalDeploymentID)
// indicates all requests for this API call are inbound

View file

@ -290,6 +290,8 @@ func errToEntry(ctx context.Context, err error, errKind ...interface{}) log.Entr
if req == nil {
req = &ReqInfo{API: "SYSTEM"}
}
req.RLock()
defer req.RUnlock()
API := "SYSTEM"
if req.API != "" {

View file

@ -41,6 +41,7 @@ type ObjectVersion struct {
}
// ReqInfo stores the request info.
// Reading/writing directly to struct requires appropriate R/W lock.
type ReqInfo struct {
RemoteHost string // Client Host/IP
Host string // Node Host/IP
@ -111,7 +112,7 @@ func (r *ReqInfo) GetTags() []KeyVal {
}
r.RLock()
defer r.RUnlock()
return append([]KeyVal(nil), r.tags...)
return append(make([]KeyVal, 0, len(r.tags)), r.tags...)
}
// GetTagsMap - returns the user defined tags in a map structure
@ -145,7 +146,6 @@ func GetReqInfo(ctx context.Context) *ReqInfo {
return r
}
r = &ReqInfo{}
SetReqInfo(ctx, r)
return r
}
return nil

View file

@ -148,7 +148,7 @@ func (c *Client) Call(ctx context.Context, method string, values url.Values, bod
atomic.AddUint64(&networkErrsCounter, 1)
}
if c.MarkOffline() {
logger.LogIf(context.Background(), fmt.Errorf("Marking %s temporary offline; caused by %w", c.url.String(), err))
logger.LogIf(ctx, fmt.Errorf("Marking %s temporary offline; caused by %w", c.url.String(), err))
}
}
return nil, &NetworkError{err}
@ -171,7 +171,7 @@ func (c *Client) Call(ctx context.Context, method string, values url.Values, bod
// fully it should make sure to respond with '412'
// instead, see cmd/storage-rest-server.go for ideas.
if c.HealthCheckFn != nil && resp.StatusCode == http.StatusPreconditionFailed {
logger.LogIf(context.Background(), fmt.Errorf("Marking %s temporary offline; caused by PreconditionFailed with disk ID mismatch", c.url.String()))
logger.LogIf(ctx, fmt.Errorf("Marking %s temporary offline; caused by PreconditionFailed with disk ID mismatch", c.url.String()))
c.MarkOffline()
}
defer xhttp.DrainBody(resp.Body)
@ -183,7 +183,7 @@ func (c *Client) Call(ctx context.Context, method string, values url.Values, bod
atomic.AddUint64(&networkErrsCounter, 1)
}
if c.MarkOffline() {
logger.LogIf(context.Background(), fmt.Errorf("Marking %s temporary offline; caused by %w", c.url.String(), err))
logger.LogIf(ctx, fmt.Errorf("Marking %s temporary offline; caused by %w", c.url.String(), err))
}
}
return nil, err