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().
---
 lib/btrfsutil/graph.go        | 115 +++++++++++++++++++++++-------------------
 lib/btrfsutil/rebuilt_tree.go |   8 ++-
 2 files changed, 65 insertions(+), 58 deletions(-)

(limited to 'lib/btrfsutil')

diff --git a/lib/btrfsutil/graph.go b/lib/btrfsutil/graph.go
index 8e26c08..860a49c 100644
--- a/lib/btrfsutil/graph.go
+++ b/lib/btrfsutil/graph.go
@@ -261,68 +261,75 @@ func (g Graph) InsertNode(node *btrfstree.Node) {
 }
 
 func (g Graph) FinalCheck(ctx context.Context, fs btrfstree.NodeSource) error {
-	var stats textui.Portion[int]
+	{
+		dlog.Info(ctx, "Checking keypointers for dead-ends...")
 
-	dlog.Info(ctx, "Checking keypointers for dead-ends...")
-	progressWriter := textui.NewProgress[textui.Portion[int]](ctx, dlog.LogLevelInfo, textui.Tunable(1*time.Second))
-	stats.D = len(g.EdgesTo)
-	progressWriter.Set(stats)
-	for laddr := range g.EdgesTo {
-		if !maps.HasKey(g.Nodes, laddr) {
-			node, err := fs.AcquireNode(ctx, laddr, btrfstree.NodeExpectations{
-				LAddr: containers.OptionalValue(laddr),
-			})
-			fs.ReleaseNode(node)
-			if err == nil {
-				progressWriter.Done()
-				return fmt.Errorf("node@%v exists but was not in node scan results", laddr)
-			}
-			g.BadNodes[laddr] = err
-		}
-		stats.N++
+		var stats textui.Portion[int]
+		stats.D = len(g.EdgesTo)
+		progressWriter := textui.NewProgress[textui.Portion[int]](ctx, dlog.LogLevelInfo, textui.Tunable(1*time.Second))
 		progressWriter.Set(stats)
-	}
-	progressWriter.Done()
-	dlog.Info(ctx, "... done checking keypointers")
 
-	dlog.Info(ctx, "Checking for btree loops...")
-	stats.D = len(g.Nodes)
-	stats.N = 0
-	progressWriter = textui.NewProgress[textui.Portion[int]](ctx, dlog.LogLevelInfo, textui.Tunable(1*time.Second))
-	progressWriter.Set(stats)
-	visited := make(containers.Set[btrfsvol.LogicalAddr], len(g.Nodes))
-	numLoops := 0
-	var checkNode func(node btrfsvol.LogicalAddr, stack []btrfsvol.LogicalAddr)
-	checkNode = func(node btrfsvol.LogicalAddr, stack []btrfsvol.LogicalAddr) {
-		defer func() {
-			stats.N = len(visited)
+		for laddr := range g.EdgesTo {
+			if !maps.HasKey(g.Nodes, laddr) {
+				node, err := fs.AcquireNode(ctx, laddr, btrfstree.NodeExpectations{
+					LAddr: containers.OptionalValue(laddr),
+				})
+				fs.ReleaseNode(node)
+				if err == nil {
+					progressWriter.Done()
+					return fmt.Errorf("node@%v exists but was not in node scan results", laddr)
+				}
+				g.BadNodes[laddr] = err
+			}
+			stats.N++
 			progressWriter.Set(stats)
-		}()
-		if visited.Has(node) {
-			return
 		}
-		if slices.Contains(node, stack) {
-			numLoops++
-			dlog.Error(ctx, "loop:")
-			for _, line := range g.renderLoop(append(stack, node)) {
-				dlog.Errorf(ctx, "    %s", line)
+		progressWriter.Done()
+		dlog.Info(ctx, "... done checking keypointers")
+	}
+
+	{
+		dlog.Info(ctx, "Checking for btree loops...")
+
+		var stats textui.Portion[int]
+		stats.D = len(g.Nodes)
+		progressWriter := textui.NewProgress[textui.Portion[int]](ctx, dlog.LogLevelInfo, textui.Tunable(1*time.Second))
+		progressWriter.Set(stats)
+
+		visited := make(containers.Set[btrfsvol.LogicalAddr], len(g.Nodes))
+		numLoops := 0
+		var checkNode func(node btrfsvol.LogicalAddr, stack []btrfsvol.LogicalAddr)
+		checkNode = func(node btrfsvol.LogicalAddr, stack []btrfsvol.LogicalAddr) {
+			defer func() {
+				stats.N = len(visited)
+				progressWriter.Set(stats)
+			}()
+			if visited.Has(node) {
+				return
+			}
+			if slices.Contains(node, stack) {
+				numLoops++
+				dlog.Error(ctx, "loop:")
+				for _, line := range g.renderLoop(append(stack, node)) {
+					dlog.Errorf(ctx, "    %s", line)
+				}
+				return
 			}
-			return
+			stack = append(stack, node)
+			for _, kp := range g.EdgesTo[node] {
+				checkNode(kp.FromNode, stack)
+			}
+			visited.Insert(node)
 		}
-		stack = append(stack, node)
-		for _, kp := range g.EdgesTo[node] {
-			checkNode(kp.FromNode, stack)
+		for _, node := range maps.SortedKeys(g.Nodes) {
+			checkNode(node, nil)
 		}
-		visited.Insert(node)
-	}
-	for _, node := range maps.SortedKeys(g.Nodes) {
-		checkNode(node, nil)
-	}
-	progressWriter.Done()
-	if numLoops > 0 {
-		return fmt.Errorf("%d btree loops", numLoops)
+		progressWriter.Done()
+		if numLoops > 0 {
+			return fmt.Errorf("%d btree loops", numLoops)
+		}
+		dlog.Info(ctx, "... done checking for loops")
 	}
-	dlog.Info(ctx, "... done checking for loops")
 
 	return nil
 }
@@ -352,6 +359,7 @@ func ReadGraph(_ctx context.Context, fs *btrfs.FS, nodeList []btrfsvol.LogicalAd
 	progressWriter.Set(stats)
 	for _, laddr := range nodeList {
 		if err := ctx.Err(); err != nil {
+			progressWriter.Done()
 			return Graph{}, err
 		}
 		node, err := fs.AcquireNode(ctx, laddr, btrfstree.NodeExpectations{
@@ -359,6 +367,7 @@ func ReadGraph(_ctx context.Context, fs *btrfs.FS, nodeList []btrfsvol.LogicalAd
 		})
 		if err != nil {
 			fs.ReleaseNode(node)
+			progressWriter.Done()
 			return Graph{}, err
 		}
 		graph.InsertNode(node)
diff --git a/lib/btrfsutil/rebuilt_tree.go b/lib/btrfsutil/rebuilt_tree.go
index ffb2e5f..31a31be 100644
--- a/lib/btrfsutil/rebuilt_tree.go
+++ b/lib/btrfsutil/rebuilt_tree.go
@@ -259,11 +259,9 @@ func (tree *RebuiltTree) items(ctx context.Context, inc bool) containers.SortedM
 			progressWriter.Set(stats)
 		}
 	}
-	if stats.Leafs.N > 0 {
-		stats.Leafs.N = stats.Leafs.D
-		progressWriter.Set(stats)
-		progressWriter.Done()
-	}
+	stats.Leafs.N = stats.Leafs.D
+	progressWriter.Set(stats)
+	progressWriter.Done()
 
 	return index
 }
-- 
cgit v1.2.3-2-g168b