fix: walk() should cancel itself upon context cancellation (#15553)

This PR fixes possible leaks that may emanate from not
listening on context cancelation or timeouts.

```
goroutine 60957610 [chan send, 16 minutes]:
github.com/minio/minio/cmd.(*erasureServerPools).Walk.func1.1.1(...)
        github.com/minio/minio/cmd/erasure-server-pool.go:1724 +0x368
github.com/minio/minio/cmd.listPathRaw({0x4a9a740, 0xc0666dffc0},...
        github.com/minio/minio/cmd/metacache-set.go:1022 +0xfc4
github.com/minio/minio/cmd.(*erasureServerPools).Walk.func1.1()
        github.com/minio/minio/cmd/erasure-server-pool.go:1764 +0x528
created by github.com/minio/minio/cmd.(*erasureServerPools).Walk.func1
        github.com/minio/minio/cmd/erasure-server-pool.go:1697 +0x1b7
```
This commit is contained in:
Harshavardhana 2022-08-18 17:49:08 -07:00 committed by GitHub
parent d350b666ff
commit e9055e9ef7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 25 additions and 23 deletions

View File

@ -2053,9 +2053,9 @@ func resyncBucket(ctx context.Context, bucket, arn string, heal bool, objectAPI
return
}
// Walk through all object versions - note ascending order of walk needed to ensure delete marker replicated to
// target after object version is first created.
if err := objectAPI.Walk(ctx, bucket, "", objInfoCh, ObjectOptions{WalkAscending: true}); err != nil {
// Walk through all object versions - Walk() is always in ascending order needed to ensure
// delete marker replicated to target after object version is first created.
if err := objectAPI.Walk(ctx, bucket, "", objInfoCh, ObjectOptions{}); err != nil {
logger.LogIf(ctx, err)
return
}

View File

@ -1871,18 +1871,17 @@ func (z *erasureServerPools) Walk(ctx context.Context, bucket, prefix string, re
cancel()
return
}
if opts.WalkAscending {
for i := len(fivs.Versions) - 1; i >= 0; i-- {
version := fivs.Versions[i]
versioned := vcfg != nil && vcfg.Versioned(version.Name)
results <- version.ToObjectInfo(bucket, version.Name, versioned)
}
return
}
versionsSorter(fivs.Versions).reverse()
for _, version := range fivs.Versions {
versioned := vcfg != nil && vcfg.Versioned(version.Name)
results <- version.ToObjectInfo(bucket, version.Name, versioned)
select {
case <-ctx.Done():
return
case results <- version.ToObjectInfo(bucket, version.Name, versioned):
}
}
}
@ -1924,6 +1923,7 @@ func (z *erasureServerPools) Walk(ctx context.Context, bucket, prefix string, re
if err := listPathRaw(ctx, lopts); err != nil {
logger.LogIf(ctx, fmt.Errorf("listPathRaw returned %w: opts(%#v)", err, lopts))
cancel()
return
}
}()

View File

@ -3007,13 +3007,13 @@ func (es *erasureSingle) Walk(ctx context.Context, bucket, prefix string, result
return err
}
vcfg, _ := globalBucketVersioningSys.Get(bucket)
ctx, cancel := context.WithCancel(ctx)
go func() {
defer cancel()
defer close(results)
versioned := opts.Versioned || opts.VersionSuspended
var wg sync.WaitGroup
wg.Add(1)
go func() {
@ -3028,15 +3028,17 @@ func (es *erasureSingle) Walk(ctx context.Context, bucket, prefix string, result
cancel()
return
}
if opts.WalkAscending {
for i := len(fivs.Versions) - 1; i >= 0; i-- {
version := fivs.Versions[i]
results <- version.ToObjectInfo(bucket, version.Name, versioned)
}
return
}
versionsSorter(fivs.Versions).reverse()
for _, version := range fivs.Versions {
results <- version.ToObjectInfo(bucket, version.Name, versioned)
versioned := vcfg != nil && vcfg.Versioned(version.Name)
select {
case <-ctx.Done():
return
case results <- version.ToObjectInfo(bucket, version.Name, versioned):
}
}
}
@ -3078,6 +3080,7 @@ func (es *erasureSingle) Walk(ctx context.Context, bucket, prefix string, result
if err := listPathRaw(ctx, lopts); err != nil {
logger.LogIf(ctx, fmt.Errorf("listPathRaw returned %w: opts(%#v)", err, lopts))
cancel()
return
}
}()

View File

@ -79,7 +79,6 @@ type ObjectOptions struct {
// mainly set for certain WRITE operations.
SkipDecommissioned bool
WalkAscending bool // return Walk results in ascending order of versions
PrefixEnabledFn func(prefix string) bool // function which returns true if versioning is enabled on prefix
// IndexCB will return any index created but the compression.