From f4d10a92abc46bf0156ff1b475304471c16405da Mon Sep 17 00:00:00 2001
From: Luke Shumaker <lukeshu@lukeshu.com>
Date: Thu, 13 Apr 2023 02:42:56 -0600
Subject: Try to find misuses of textui.Progress

 - Add a runtime-check to Progress to notice if we deadlocked or
   forgot to call .Done().

 - Add a runtime-check to Progress.Done() to panic if .Set() was never
   called (instead of the old behavior of deadlocking).

 - grep: Use `defer` when possible, to help remember to call .Done().

 - grep: Always either call .Set() right away, or right before calling
   .Done().
---
 cmd/btrfs-rec/inspect/rebuildtrees/rebuild.go | 28 +++++++++++++--------------
 cmd/btrfs-rec/inspect/rebuildtrees/scan.go    |  2 ++
 2 files changed, 16 insertions(+), 14 deletions(-)

(limited to 'cmd')

diff --git a/cmd/btrfs-rec/inspect/rebuildtrees/rebuild.go b/cmd/btrfs-rec/inspect/rebuildtrees/rebuild.go
index 427070a..e6345ae 100644
--- a/cmd/btrfs-rec/inspect/rebuildtrees/rebuild.go
+++ b/cmd/btrfs-rec/inspect/rebuildtrees/rebuild.go
@@ -190,12 +190,12 @@ func (o *rebuilder) processAddedItemQueue(ctx context.Context) error {
 	var progress settleItemStats
 	progress.D = len(queue)
 	progressWriter := textui.NewProgress[settleItemStats](ctx, dlog.LogLevelInfo, textui.Tunable(1*time.Second))
-	ctx = dlog.WithField(ctx, "btrfs.inspect.rebuild-trees.rebuild.substep.progress", &progress)
+	progressWriter.Set(progress)
+	defer progressWriter.Done()
 
-	for i, key := range queue {
-		progress.N = i
-		progressWriter.Set(progress)
+	ctx = dlog.WithField(ctx, "btrfs.inspect.rebuild-trees.rebuild.substep.progress", &progress)
 
+	for _, key := range queue {
 		ctx := dlog.WithField(ctx, "btrfs.inspect.rebuild-trees.rebuild.settle.item", key)
 		tree := o.rebuilt.RebuiltTree(ctx, key.TreeID)
 		incPtr, ok := tree.RebuiltAcquireItems(ctx).Load(key.Key)
@@ -213,15 +213,14 @@ func (o *rebuilder) processAddedItemQueue(ctx context.Context) error {
 			o.wantAugment(ctx, wantKey, tree.RebuiltLeafToRoots(ctx, excPtr.Node))
 			progress.NumAugments = o.numAugments
 			progress.NumAugmentTrees = len(o.augmentQueue)
-			progressWriter.Set(progress)
 		} else if !btrfscheck.HandleItemWouldBeNoOp(key.ItemType) {
 			o.settledItemQueue.Insert(key)
 		}
+
+		progress.N++
+		progressWriter.Set(progress)
 	}
 
-	progress.N = len(queue)
-	progressWriter.Set(progress)
-	progressWriter.Done()
 	return nil
 }
 
@@ -251,6 +250,9 @@ func (o *rebuilder) processSettledItemQueue(ctx context.Context) error {
 	var progress processItemStats
 	progress.D = len(queue)
 	progressWriter := textui.NewProgress[processItemStats](ctx, dlog.LogLevelInfo, textui.Tunable(1*time.Second))
+	progressWriter.Set(progress)
+	defer progressWriter.Done()
+
 	ctx = dlog.WithField(ctx, "btrfs.inspect.rebuild-trees.rebuild.substep.progress", &progress)
 
 	type keyAndBody struct {
@@ -278,7 +280,6 @@ func (o *rebuilder) processSettledItemQueue(ctx context.Context) error {
 		return nil
 	})
 	grp.Go("cpu", func(ctx context.Context) error {
-		defer progressWriter.Done()
 		o.curKey.Key.OK = true
 		for item := range itemChan {
 			ctx := dlog.WithField(ctx, "btrfs.inspect.rebuild-trees.rebuild.process.item", item.keyAndTree)
@@ -323,24 +324,23 @@ func (o *rebuilder) processAugmentQueue(ctx context.Context) error {
 	runtime.GC()
 
 	progressWriter := textui.NewProgress[textui.Portion[int]](ctx, dlog.LogLevelInfo, textui.Tunable(1*time.Second))
+	progressWriter.Set(progress)
+	defer progressWriter.Done()
+
 	ctx = dlog.WithField(ctx, "btrfs.inspect.rebuild-trees.rebuild.substep.progress", &progress)
 	for _, treeID := range maps.SortedKeys(resolvedAugments) {
 		ctx := dlog.WithField(ctx, "btrfs.inspect.rebuild-trees.rebuild.augment.tree", treeID)
 		for _, nodeAddr := range maps.SortedKeys(resolvedAugments[treeID]) {
 			if err := ctx.Err(); err != nil {
-				progressWriter.Set(progress)
-				progressWriter.Done()
 				return err
 			}
-			progressWriter.Set(progress)
 			// This will call o.AddedItem as nescessary, which
 			// inserts to o.addedItemQueue.
 			o.rebuilt.RebuiltTree(ctx, treeID).RebuiltAddRoot(ctx, nodeAddr)
 			progress.N++
+			progressWriter.Set(progress)
 		}
 	}
-	progressWriter.Set(progress)
-	progressWriter.Done()
 
 	return nil
 }
diff --git a/cmd/btrfs-rec/inspect/rebuildtrees/scan.go b/cmd/btrfs-rec/inspect/rebuildtrees/scan.go
index f266dab..b1469ff 100644
--- a/cmd/btrfs-rec/inspect/rebuildtrees/scan.go
+++ b/cmd/btrfs-rec/inspect/rebuildtrees/scan.go
@@ -73,6 +73,7 @@ func ScanDevices(_ctx context.Context, fs *btrfs.FS, nodeList []btrfsvol.Logical
 	progressWriter.Set(stats)
 	for _, laddr := range nodeList {
 		if err := ctx.Err(); err != nil {
+			progressWriter.Done()
 			return ScanDevicesResult{}, err
 		}
 		node, err := fs.AcquireNode(ctx, laddr, btrfstree.NodeExpectations{
@@ -80,6 +81,7 @@ func ScanDevices(_ctx context.Context, fs *btrfs.FS, nodeList []btrfsvol.Logical
 		})
 		if err != nil {
 			fs.ReleaseNode(node)
+			progressWriter.Done()
 			return ScanDevicesResult{}, err
 		}
 		ret.insertNode(node)
-- 
cgit v1.2.3-2-g168b