go/analysis/passes/modernize: fix the potential deadlock issues in bloop
For golang/go#75599
Change-Id: Ic8009f9a7bcbd8f76b47bf6831132aeb0cbee2f3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/711980
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Bypass: Alan Donovan <adonovan@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
diff --git a/go/analysis/passes/modernize/bloop.go b/go/analysis/passes/modernize/bloop.go
index eb1ac17..bb8cf72 100644
--- a/go/analysis/passes/modernize/bloop.go
+++ b/go/analysis/passes/modernize/bloop.go
@@ -189,6 +189,7 @@
// 2. The only b.N loop in that benchmark function
// - b.Loop() can only be called once per benchmark execution
// - Multiple calls result in "B.Loop called with timer stopped" error
+// - Multiple loops may have complex interdependencies that are hard to analyze
func usesBenchmarkNOnce(c inspector.Cursor, info *types.Info) bool {
// Find the enclosing benchmark function
curFunc, ok := enclosingFunc(c)
@@ -205,17 +206,14 @@
return false
}
- // Count b.N references in this benchmark function
+ // Count all b.N references in this benchmark function (including nested functions)
bnRefCount := 0
- filter := []ast.Node{(*ast.SelectorExpr)(nil), (*ast.FuncLit)(nil)}
+ filter := []ast.Node{(*ast.SelectorExpr)(nil)}
curFunc.Inspect(filter, func(cur inspector.Cursor) bool {
- switch n := cur.Node().(type) {
- case *ast.FuncLit:
- return false // don't descend into nested function literals
- case *ast.SelectorExpr:
- if n.Sel.Name == "N" && typesinternal.IsPointerToNamed(info.TypeOf(n.X), "testing", "B") {
- bnRefCount++
- }
+ sel := cur.Node().(*ast.SelectorExpr)
+ if sel.Sel.Name == "N" &&
+ typesinternal.IsPointerToNamed(info.TypeOf(sel.X), "testing", "B") {
+ bnRefCount++
}
return true
})
diff --git a/go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go b/go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go
index 2dc71fc..33ffd77 100644
--- a/go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go
+++ b/go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go
@@ -113,3 +113,21 @@
for i := 0; i < b.N; i++ { // nope: b.N accessed more than once in benchmark
}
}
+
+func BenchmarkJ(b *testing.B) {
+ var wg sync.WaitGroup
+ ch := make(chan int, 10)
+ wg.Add(1)
+ go func() {
+ for i := 0; i < b.N; i++ {
+ <-ch
+ }
+ wg.Done()
+ }()
+ b.ResetTimer()
+ for i := 0; i < b.N; i++ { // nope: multiple b.N loops in same function
+ ch <- i
+ }
+ b.StopTimer()
+ wg.Wait()
+}
diff --git a/go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go.golden b/go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go.golden
index 3def4a0..a2def76 100644
--- a/go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go.golden
+++ b/go/analysis/passes/modernize/testdata/src/bloop/bloop_test.go.golden
@@ -111,3 +111,21 @@
for i := 0; i < b.N; i++ { // nope: b.N accessed more than once in benchmark
}
}
+
+func BenchmarkJ(b *testing.B) {
+ var wg sync.WaitGroup
+ ch := make(chan int, 10)
+ wg.Add(1)
+ go func() {
+ for i := 0; i < b.N; i++ {
+ <-ch
+ }
+ wg.Done()
+ }()
+ b.ResetTimer()
+ for i := 0; i < b.N; i++ { // nope: multiple b.N loops in same function
+ ch <- i
+ }
+ b.StopTimer()
+ wg.Wait()
+}
