From e9055e9ef73501904ad964f1e175bf476cf7d46d Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Thu, 18 Aug 2022 17:49:08 -0700
Subject: [PATCH] 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
```
---
 cmd/bucket-replication.go   |  6 +++---
 cmd/erasure-server-pool.go  | 18 +++++++++---------
 cmd/erasure-single-drive.go | 23 +++++++++++++----------
 cmd/object-api-interface.go |  1 -
 4 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go
index 0e2e626bb..3f8efd127 100644
--- a/cmd/bucket-replication.go
+++ b/cmd/bucket-replication.go
@@ -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
 	}
diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go
index e53a6be72..19dcfe9c0 100644
--- a/cmd/erasure-server-pool.go
+++ b/cmd/erasure-server-pool.go
@@ -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
 					}
 				}()
diff --git a/cmd/erasure-single-drive.go b/cmd/erasure-single-drive.go
index 6539ac042..bf2bca600 100644
--- a/cmd/erasure-single-drive.go
+++ b/cmd/erasure-single-drive.go
@@ -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
 			}
 		}()
diff --git a/cmd/object-api-interface.go b/cmd/object-api-interface.go
index 12c982ccd..e20e62338 100644
--- a/cmd/object-api-interface.go
+++ b/cmd/object-api-interface.go
@@ -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.