fix a crash from unstable sort for > 2 pools (#12501)

Fix in https://github.com/minio/minio/pull/12487 assumes that slices with 
tiebreaks are sorted equally. That is only the case for "stable"  sort versions.
This commit is contained in:
Klaus Post 2021-06-14 20:00:13 +02:00 committed by GitHub
parent 31971906ff
commit b89c0beea4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -617,19 +617,22 @@ func (z *erasureServerPools) GetObjectNInfo(ctx context.Context, bucket, object
unlockOnDefer = true unlockOnDefer = true
} }
errs := make([]error, len(z.serverPools))
grs := make([]*GetObjectReader, len(z.serverPools))
lockType = noLock // do not take locks at lower levels lockType = noLock // do not take locks at lower levels
checkPrecondFn := opts.CheckPrecondFn checkPrecondFn := opts.CheckPrecondFn
opts.CheckPrecondFn = nil opts.CheckPrecondFn = nil
results := make([]struct {
zIdx int
gr *GetObjectReader
err error
}, len(z.serverPools))
var wg sync.WaitGroup var wg sync.WaitGroup
for i, pool := range z.serverPools { for i, pool := range z.serverPools {
wg.Add(1) wg.Add(1)
go func(i int, pool *erasureSets) { go func(i int, pool *erasureSets) {
defer wg.Done() defer wg.Done()
grs[i], errs[i] = pool.GetObjectNInfo(ctx, bucket, object, rs, h, lockType, opts) results[i].zIdx = i
results[i].gr, results[i].err = pool.GetObjectNInfo(ctx, bucket, object, rs, h, lockType, opts)
}(i, pool) }(i, pool)
} }
wg.Wait() wg.Wait()
@ -638,59 +641,62 @@ func (z *erasureServerPools) GetObjectNInfo(ctx context.Context, bucket, object
// this is a defensive change to handle any duplicate // this is a defensive change to handle any duplicate
// content that may have been created, we always serve // content that may have been created, we always serve
// the latest object. // the latest object.
less := func(i, j int) bool { sort.Slice(results, func(i, j int) bool {
var ( var mtimeI, mtimeJ time.Time
mtime1 time.Time
mtime2 time.Time if results[i].gr != nil {
) mtimeI = results[i].gr.ObjInfo.ModTime
if grs[i] != nil {
mtime1 = grs[i].ObjInfo.ModTime
} }
if grs[j] != nil { if results[j].gr != nil {
mtime2 = grs[j].ObjInfo.ModTime mtimeJ = results[j].gr.ObjInfo.ModTime
} }
return mtime1.After(mtime2) // On tiebreaks, choose the earliest.
if mtimeI.Equal(mtimeJ) {
return results[i].zIdx < results[j].zIdx
} }
sort.Slice(errs, less) return mtimeI.After(mtimeJ)
sort.Slice(grs, less) })
var found = -1 var found = -1
for i, err := range errs { for i, res := range results {
if err == nil { if res.err == nil {
// Select earliest result
found = i found = i
break break
} }
if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) { if !isErrObjectNotFound(res.err) && !isErrVersionNotFound(res.err) {
for _, grr := range grs { for _, res := range results {
if grr != nil { res.gr.Close()
grr.Close()
} }
} return nil, res.err
return gr, err
} }
} }
if found >= 0 { if found < 0 {
if checkPrecondFn != nil && checkPrecondFn(grs[found].ObjInfo) {
for _, grr := range grs {
grr.Close()
}
return nil, PreConditionFailed{}
}
for i, grr := range grs {
if i == found {
continue
}
grr.Close()
}
return grs[found], nil
}
object = decodeDirObject(object) object = decodeDirObject(object)
if opts.VersionID != "" { if opts.VersionID != "" {
return gr, VersionNotFound{Bucket: bucket, Object: object, VersionID: opts.VersionID} return gr, VersionNotFound{Bucket: bucket, Object: object, VersionID: opts.VersionID}
} }
return gr, ObjectNotFound{Bucket: bucket, Object: object} return gr, ObjectNotFound{Bucket: bucket, Object: object}
}
// Check preconditions.
if checkPrecondFn != nil && checkPrecondFn(results[found].gr.ObjInfo) {
for _, res := range results {
res.gr.Close()
}
return nil, PreConditionFailed{}
}
// Close all others
for i, res := range results {
if i == found {
continue
}
res.gr.Close()
}
return results[found].gr, nil
} }
func (z *erasureServerPools) GetObjectInfo(ctx context.Context, bucket, object string, opts ObjectOptions) (objInfo ObjectInfo, err error) { func (z *erasureServerPools) GetObjectInfo(ctx context.Context, bucket, object string, opts ObjectOptions) (objInfo ObjectInfo, err error) {
@ -713,15 +719,19 @@ func (z *erasureServerPools) GetObjectInfo(ctx context.Context, bucket, object s
ctx = lkctx.Context() ctx = lkctx.Context()
defer lk.RUnlock(lkctx.Cancel) defer lk.RUnlock(lkctx.Cancel)
errs := make([]error, len(z.serverPools)) results := make([]struct {
objInfos := make([]ObjectInfo, len(z.serverPools)) zIdx int
oi ObjectInfo
err error
}, len(z.serverPools))
opts.NoLock = true // avoid taking locks at lower levels for multi-pool setups. opts.NoLock = true // avoid taking locks at lower levels for multi-pool setups.
var wg sync.WaitGroup var wg sync.WaitGroup
for i, pool := range z.serverPools { for i, pool := range z.serverPools {
wg.Add(1) wg.Add(1)
go func(i int, pool *erasureSets) { go func(i int, pool *erasureSets) {
defer wg.Done() defer wg.Done()
objInfos[i], errs[i] = pool.GetObjectInfo(ctx, bucket, object, opts) results[i].zIdx = i
results[i].oi, results[i].err = pool.GetObjectInfo(ctx, bucket, object, opts)
}(i, pool) }(i, pool)
} }
wg.Wait() wg.Wait()
@ -730,31 +740,27 @@ func (z *erasureServerPools) GetObjectInfo(ctx context.Context, bucket, object s
// this is a defensive change to handle any duplicate // this is a defensive change to handle any duplicate
// content that may have been created, we always serve // content that may have been created, we always serve
// the latest object. // the latest object.
less := func(i, j int) bool { sort.Slice(results, func(i, j int) bool {
mtime1 := objInfos[i].ModTime a, b := results[i], results[j]
mtime2 := objInfos[j].ModTime if a.oi.ModTime.Equal(b.oi.ModTime) {
return mtime1.After(mtime2) // On tiebreak, select the lowest zone index.
return a.zIdx < b.zIdx
} }
sort.Slice(errs, less) return a.oi.ModTime.After(b.oi.ModTime)
sort.Slice(objInfos, less) })
for _, res := range results {
var found = -1 // Return first found.
for i, err := range errs { err := res.err
if err == nil { if err == nil {
found = i return res.oi, nil
break
} }
if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) { if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
// some errors such as MethodNotAllowed for delete marker // some errors such as MethodNotAllowed for delete marker
// should be returned upwards. // should be returned upwards.
return objInfos[i], err return res.oi, err
} }
} }
if found >= 0 {
return objInfos[found], nil
}
object = decodeDirObject(object) object = decodeDirObject(object)
if opts.VersionID != "" { if opts.VersionID != "" {
return objInfo, VersionNotFound{Bucket: bucket, Object: object, VersionID: opts.VersionID} return objInfo, VersionNotFound{Bucket: bucket, Object: object, VersionID: opts.VersionID}